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

Lilly C16 Pine #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions heaps/heap_sort.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@

def heapify(arr, n, i):
largest = i
l = 2 * i + 1
r = 2 * i + 2
if l < n and arr[l] > arr[largest]:
largest = l
if r < n and arr[r] > arr[largest]:
largest = r
if largest != i:
arr[i], arr[largest] = arr[largest], arr[i]
heapify(arr, n, largest)

Choose a reason for hiding this comment

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

Consider reworking this to be iterative. As written, this is basically a max heap heap_down (pushes down smaller numbers). The addition of the n (size) parameter lets you arbitrarily set what the "end" of the heap is, allowing the data array to be segmented into a heap portion, and a non-heap (becomes the sorted) portion. But since this is written recursively, we can't so better than O(log n) for time and space for any given call (the log n space comes from recursive call stacks).

If implemented iteratively, this would have time complexity O(log n), but space complexity O(1).

Alternatively, it's not too much work to start from the MinHeap implementation, but remove the need for the HeapNode, and initialize the store to use the list to be sorted directly. Switching to a max heap is a great idea (to sort ascending), and only a small change would be required to switch from the min heap to a max heap.


def heap_sort(list):
def heap_sort(arr):
""" This method uses a heap to sort an array.
Time Complexity: ?
Space Complexity: ?
Time Complexity: O(n)
Space Complexity: O(n)
Comment on lines +16 to +17

Choose a reason for hiding this comment

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

👀 Since each call to your heapify helper takes O(log n) time, and we are calling it 2n times here, the time complexity of heap_sort will be O(2n log n) → O(n log n). Note that for a general sorting algorithm (like heap sort), O(n log n) is the theoretical best we can do!

For space complexity, you avoided needing to allocate the internal store that MinHeap used, but since heapify is implemented recursively, the stack will grow with the number of layers in the heap. So each heapify call will have a stack with O(log n) levels. Each is used only for the duration of a single heapify call, so they aren't cumulative, resulting in an overall space complexity of O(log n). But if you modified your heapify to be iterative, the space complexity would be constant O(1)!

"""
pass
n = len(arr)
for i in range(n, -1, -1):
heapify(arr, n, i)
for i in range(n - 1, 0, -1):
arr[i], arr[0] = arr[0], arr[i]
heapify(arr, i, 0)
Comment on lines +20 to +24

Choose a reason for hiding this comment

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

Nice approach of building a max heap in place (though with the additional stack cost), and then moving the max value in the heap to end end of the list, heapifying the remaining data.

return arr

# time complexity: O(nlog(n))
# space complexity: O(1)
63 changes: 49 additions & 14 deletions heaps/min_heap.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,34 @@ def __init__(self):
def add(self, key, value = None):
""" This method adds a HeapNode instance to the heap
If value == None the new node's value should be set to key
Time Complexity: ?
Space Complexity: ?
Time Complexity: O(log n)
Space Complexity: O(1)
Comment on lines +22 to +23

Choose a reason for hiding this comment

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

✨ Great! 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). Your implementation of the heap_up helper is iterative, meaning that while we do have to loop log n times, we don't have any recursive stack overhead. So the space complexity is constant (O(1)).

"""
pass
if value == None:

Choose a reason for hiding this comment

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

👀 Prefer using is to compare to None

value = key

self.store.append(HeapNode(key, value))
self.heap_up(len(self.store) - 1)

# this method removes heap node with minimum key
def remove(self):
""" This method removes and returns an element from the heap
maintaining the heap structure
Time Complexity: ?
Space Complexity: ?
Time Complexity: O(log n)
Space Complexity: O(1)
Comment on lines +35 to +36

Choose a reason for hiding this comment

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

✨ Nice. Just as for add, the complexities are really coming from the heap_down implementation. And like your heap_up, by taking an iterative approach, you achieve O(1) space complexity.

"""
pass
if len(self.store) == 0:

Choose a reason for hiding this comment

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

👀 We have an empty helper we could use here

return None

first_node = 0
last_node = len(self.store) - 1

self.swap(first_node, last_node)
result = self.store.pop()

self.heap_down(first_node)

return result.value

def __str__(self):
""" This method lets you print the heap, when you're testing your app.
Expand All @@ -44,11 +58,10 @@ def __str__(self):

def empty(self):
""" This method returns true if the heap is empty
Time complexity: ?
Space complexity: ?
Time complexity: O(1)
Space complexity: O(1)
Comment on lines +61 to +62

Choose a reason for hiding this comment

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

"""
pass

return len(self.store) == 0

Choose a reason for hiding this comment

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

Remember that an empty list is falsy

        return not self.store


def heap_up(self, index):
""" This helper method takes an index and
Expand All @@ -57,18 +70,40 @@ def heap_up(self, index):
property is reestablished.

This could be **very** helpful for the add method.
Time complexity: ?
Space complexity: ?
Time complexity: O(log n)
Space complexity: O(1)
Comment on lines +73 to +74

Choose a reason for hiding this comment

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

"""
pass
temporary = index

Choose a reason for hiding this comment

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

You could have used index directly without copying it into temporary. Since we passed in an integer (immutable), we can modify the value within the function without worrying about it affecting the original value that was passed in.

while temporary > 0:
parent = (temporary - 1) // 2
if self.store[temporary].key < self.store[parent].key:
self.swap(temporary, parent)
temporary = parent
else:
break
Comment on lines +79 to +83

Choose a reason for hiding this comment

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

Consider reversing the sense of the comparison to treat it more like a guard clause, which helps avoid the dangling else and lets us unindent the main logic.

Suggested change
if self.store[temporary].key < self.store[parent].key:
self.swap(temporary, parent)
temporary = parent
else:
break
if self.store[temporary].key >= self.store[parent].key:
break
self.swap(temporary, parent)
temporary = parent



# method to move heap down
def heap_down(self, index):

Choose a reason for hiding this comment

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

✨ Nice approach of first determining the candidate child, then deciding whether the swap is required.

Though not prompted, like heap_up, heap_down is also O(log n) in time and constant in space complexity. The worst case for re-heapifying is if the new root need to move back down to a leaf, requiring log n iterations. And since you took the iterative approach, there's no additional stack growth, leaving us with constant space.

""" 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
temporary = index
while temporary < len(self.store) - 1:
left_child = 2 * temporary + 1
right_child = 2 * temporary + 2
if left_child < len(self.store) and self.store[temporary].key > self.store[left_child].key:
temporary = left_child
if right_child < len(self.store) and self.store[temporary].key > self.store[right_child].key:
temporary = right_child
if temporary == index:
break
self.swap(temporary, index)
index = temporary
Comment on lines +101 to +104

Choose a reason for hiding this comment

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

✨ This mirrors my suggestion for how to rewrite the condition in heap_up. Notice how we get to unindent the main logic here as well!





def swap(self, index_1, index_2):
Expand Down