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

Ports - Amy W #30

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Ports - Amy W #30

wants to merge 5 commits into from

Conversation

amythetester
Copy link

No description provided.

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.

Not bad, you made a few mistakes with the Big-O and one method delete doesn't work, but otherwise you did quite well. Check out my comments and let me know if you have any questions.

# method to delete the first node found with specified value
# Time Complexity: O(n) where n is the length of the linke list
# Space Complexity: O(1)
def delete(value)

Choose a reason for hiding this comment

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

This one isn't working at all.

Here's a couple of suggestions:

  1. You're using @head in the until loop. You shouldn't do that in the as current will be moving through the list, well past @head.
  2. You are doing last.next.next without checking to ensure that last.next is not nil.

You should also have a previous variable to keep track of the node before current.

# Space Complexity
def has_cycle
raise NotImplementedError
while fast != nil && fast.next != nil

Choose a reason for hiding this comment

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

I like how you're doing it without having to find the entire length first!

# assume indexing starts at 0 while counting to n
# Time Complexity: O(n) where n is the length of the linked list
# Space Complexity: O(1)
def find_nth_from_end(n)

Choose a reason for hiding this comment

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

Could you do this one in a similar manner to has_cycle so that you don't have to find the length before finding the nth from the end?

# linked list links to a node already visited.
# returns true if a cycle is found, false otherwise.
# Time Complexity: O(n) where n is the length of the linked list
# Space Complexity: O(1)

Choose a reason for hiding this comment

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

Space complexity of O(1)? Really? You're making an array visited which will get as large as the list.

You are also using visited.include?(current) which will do a loop through the array to find current. So you have a loop in a loop or an O(n2) algorithm.

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