Leaves_Ga-Young #32
Open
Leaves_Ga-Young #32
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CheezItMan
reviewed
Apr 17, 2020
lib/heap_sort.rb
Outdated
| end | ||
|
|
||
| # I'm having trouble figuring out why the "can sort a 5-element array" test will not pass for this. I have created a new "can sort a different 5-element array" test and that passes. I am thinking it has to do with the order of numbers - some exact pattern of numbers is causing the heap to not sort properly, but all my tests from the min_heap are passing. Could I get some help? |
lib/min_heap.rb
Outdated
Comment on lines
17
to
19
| # Time Complexity: O(log n) where n is number of nodes | ||
| # Space Complexity: O(1) | ||
| def add(key, value) |
There was a problem hiding this comment.
Since heap_up is a recursive method and it will make log n recursive calls. This method has O(log n) space complexity.
lib/min_heap.rb
Outdated
Comment on lines
27
to
29
| # Time Complexity: O(log n) where n is number of nodes | ||
| # Space Complexity: O(1) | ||
| def remove() |
There was a problem hiding this comment.
- Similar issue to above on space complexity.
- What if the heap is empty? maybe a check is appropriate.
Comment on lines
+56
to
58
| # Time complexity: O(1) | ||
| # Space complexity: O(1) | ||
| def empty? |
lib/min_heap.rb
Outdated
Comment on lines
75
to
77
| else | ||
| return | ||
| end |
There was a problem hiding this comment.
This is a bit redundant.
Suggested change
| else | |
| return | |
| end |
lib/min_heap.rb
Outdated
| left_child = (index * 2 + 1) | ||
| right_child = (index * 2 + 2) | ||
|
|
||
| until @store[left_child] == nil || @store[right_child] == nil |
There was a problem hiding this comment.
What if the left child is not nil but the right child is, and yet if the parent is still bigger than the left child.
Comment on lines
-65
to
85
| # moves it up the heap if it's smaller | ||
| # moves it down the heap if it's smaller | ||
| # than it's parent node. | ||
| def heap_down(index) |
There was a problem hiding this comment.
Ok this one has a small bug that heapsort caught. Take a look at it, you can get into the circumstance that the left child is not nil and smaller than the parent and the right child is nil.
|
Hi Chris, I think I fixed that bug. Could you take another look? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Heaps Practice
Congratulations! You're submitting your assignment!
Comprehension Questions
heap_up&heap_downmethods useful? Why?