Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added longest prefix, needs work to pass test #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Steph0088
Copy link

Both my methods need work to pass...this is what I have so far.

Copy link

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well both methods aren't working. If you have time lets sit down and look at things a bit. Your 1st method remove_duplicates is close to working, while longest prefix has more issues. See my comments and see what you can work out.


while list.length >= i
if compare_to == list[i]
list.delete_at(list[i])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete_at takes an index number as an argument not an element. So that's why this isn't working.

delete_at also shifts each subsequent element over one index so it's an O(n) method. Since it's nested inside a loop which runs n times, this means the method would be O(n2)

Also if you delete an element you should decrement i by one so that you don't end up skipping any duplicates.

def longest_prefix(strings)
raise NotImplementedError, "Not implemented yet"

common_word = ""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are going to need 2 loops here one of which is looping through the characters inside one of the strings. You only have one loop here and you're comparing using an index for the array as an index for a letter in a word.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants