Cedar - Kristin L #30
Conversation
There was a problem hiding this comment.
💫 Overall, a nice implementation, Kristin. However, the implementation of heap_down appears to have come from the solution. As such, I can't evaluate that implementation. Also, the comprehension questions are missing, so I'm giving an overall evaluation of yellow.
Please let me know if you'd like to discuss this further, or feel free to submit a new implementation.
🟡
| """ | ||
| pass | ||
| if value == None: |
There was a problem hiding this comment.
👀 Prefer using is to compare to None
| Time Complexity: O(log n) | ||
| Space Complexity: O(1) |
There was a problem hiding this comment.
👀 In the worst case, the new value we're inserting is the new root of the heap, meaning it would need to move up the full height of the heap (which is log n levels deep) leading to O(log n) time complexity. Your implementation of the heap_up helper is recursive, meaning that for each recursive call (up to log n of them) there is stack space being consumed. So the space complexity would also be O(log n). If heap_up were implemented iteratively, this could be reduced to O(1) space complexity.
| Time Complexity: O(log n) | ||
| Space Complexity: O(1) |
There was a problem hiding this comment.
👀 In the worst case, the value that got swapped to the top will need to move all the way back down to a leaf, meaning it would need to move down the full height of the heap (which is log n levels deep) leading to O(log n) time complexity. Your implementation of the heap_down helper is recursive, meaning that for each recursive call (up to log n of them) there is stack space being consumed. So the space complexity would also be O(log n). If heap_down were implemented iteratively, this could be reduced to O(1) space complexity.
| """ | ||
| pass | ||
| if self.empty(): |
There was a problem hiding this comment.
✨ Nice use of your own helper method!
| Time complexity: O(n) | ||
| Space complexity: O(1) |
There was a problem hiding this comment.
👀 Getting the length of a list is a constant time operation, so the overall time complexity will be constant time as well.
| if len(self.store) == 0: | ||
| return True |
There was a problem hiding this comment.
👀 This will return True when the list is empty, but what would happens if the list were not empty?
Python would fall off the end of the function, with the default None return value. But since we're returning a boolean from one path, we should be consistent everywhere.
if len(self.store) == 0:
return True
else:
return Falsewhich simplifies to
return len(self.store) == 0We could also remember that empty lists are falsy, and write this as
return not self.store| check my value vs parent's value | ||
| if parent is larger | ||
| swap with parent | ||
| recursively call heap_up from new location |
There was a problem hiding this comment.
✨ Nice explanation of the approach
| Time complexity: O(log n) | ||
| Space complexity: O(1) |
There was a problem hiding this comment.
👀 This function is the main source of time and space complexity for add, so refer back to that note.
|
|
||
| def heap_down(self, index): | ||
| """ This helper method takes an index and | ||
| moves the corresponding element down the heap if it's | ||
| larger than either of its children and continues until | ||
| the heap property is reestablished. | ||
| """ | ||
| pass | ||
| left_child = index * 2 + 1 |
There was a problem hiding this comment.
👀 This is identical to the instructor solution and has no additions that would indicate to me that time was spent time understanding this approach.
| Time Complexity: O(n log n) | ||
| Space Complexity: O(n) |
There was a problem hiding this comment.
✨ Great. Since sorting using a heap reduces down to building up a heap of n items one-by-one (each taking O(log n)), then pulling them back out again (again taking O(log n) for each of n items), we end up with a time complexity of O(2n log n) → O(n log n). While for the space, we do need to worry about the O(log n) space consume during each add and remove, but they aren't cumulative (each is consumed only during the call to add or remove). However, the internal store for the MinHeap does grow with the size of the input list. So the maximum space would be O(n + log n) → O(n), since n is a larger term than log n.
Note that a fully in-place solution (O(1) space complexity) would require both avoiding the recursive calls, as well as working directly with the originally provided list (no internal store).
| while not heap.empty(): | ||
| # puts sorted nums back in list using index | ||
| # remove is O(log n) | ||
| list[index] = heap.remove() | ||
| index += 1 | ||
|
|
||
| return list |
There was a problem hiding this comment.
Note the since this isn't a fully in-place solution (the MinHeap has a O(n) internal store), we don't necessarily need to modify the passed in list. The tests are written to check the return value, so we could unpack the heap into a new result list to avoid mutating the input.
result = []
while not heap.empty():
result.append(heap.remove())
return result
Heaps Practice
Congratulations! You're submitting your assignment!
Comprehension Questions
heap_up&heap_downmethods useful? Why?