-
Notifications
You must be signed in to change notification settings - Fork 37
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
Fire - Jing #20
base: master
Are you sure you want to change the base?
Fire - Jing #20
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It passes all the tests, but it doesn't do what it's supposed to do. Take a look at my comments and let me know what questions you have.
There is some good code here, but a lot is just... rough, maybe make a 2nd stab at this.
def heapsort(list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also you add things into a heap, but in the internal structure the items are not sorted. They're maintained in heap order with the parent smaller than it's children. You have to REMOVE elements from the heap 1-by-1 to be guaranteed to get it into order.
# Time Complexity: O(logn) | ||
# Space Complexity: O(1) | ||
def add(key, value = key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
lib/min_heap.rb
Outdated
if @store.empty? | ||
@store << HeapNode.new(key, value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need this if block?
lib/min_heap.rb
Outdated
def remove() | ||
raise NotImplementedError, "Method not implemented yet..." | ||
min = @store.min_by {|node| node.key} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a linear search, you're totally losing the advantage of a Heap here!
# Time complexity: O(0) | ||
# Space complexity: O(0) | ||
def empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
# Time complexity: O(logn) | ||
# Space complexity: O(0) | ||
def heap_up(index) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
# This helper method takes an index and | ||
# moves it up the heap if it's smaller | ||
# than it's parent node. | ||
def heap_down(index) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't working because you've got to:
- Check the both left and right child
- And swap with the smaller of the two, if needed
- Consider the case where there is a left child and not a right child
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
swap(index, index * 2 + 1) | ||
swap(index, index * 2 + 2) if @store[index * 2 + 2] != nil && @store[index].key > @store[index * 2 + 2].key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!
Heaps Practice
Congratulations! You're submitting your assignment!
Comprehension Questions
heap_up
&heap_down
methods useful? Why?