Skip to content

Test passing #39

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Test passing #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 16 additions & 4 deletions heaps/heap_sort.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,20 @@

from heaps.min_heap import MinHeap

def heap_sort(list):
""" This method uses a heap to sort an array.
Time Complexity: ?
Space Complexity: ?
Time Complexity: (log n)
Space Complexity: (O n)
Comment on lines +5 to +6
Copy link

Choose a reason for hiding this comment

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

👀 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 should be aware of the O(log n) space consume during each add and remove, but they aren't cumulative (each is consumed only during the individual 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).

"""
pass

heap = MinHeap()

for i in list:
heap.add(i)

i = 0

while not heap.empty():
list[i] = heap.remove()
i += 1
Comment on lines +14 to +18
Copy link

Choose a reason for hiding this comment

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

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


return list
59 changes: 46 additions & 13 deletions heaps/min_heap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,28 @@ 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: ? (log n)
Space Complexity: (O 1)
Comment on lines +22 to +23
Copy link

Choose a reason for hiding this comment

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

👀 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.

"""
pass
if value == None:
Copy link

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
node = HeapNode(key, value)
self.store.append(node)
self.heap_up(len(self.store) - 1)

def remove(self):
""" This method removes and returns an element from the heap
maintaining the heap structure
Time Complexity: ?
Space Complexity: ?
Time Complexity: (O n)
Space Complexity: (O N)
Comment on lines +34 to +35
Copy link

Choose a reason for hiding this comment

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

👀 Similar to add, 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 len(self.store) == 0:
Copy link

Choose a reason for hiding this comment

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

👀 We have a helper method that can check for the heap being empty.

return None
self.swap(0, len(self.store) - 1)
min = self.store.pop()
self.heap_down(0)

return min.value



Expand All @@ -44,10 +54,11 @@ 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 +57 to +58
Copy link

Choose a reason for hiding this comment

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

✨

"""
pass
if len(self.store) == 0:
return True
Comment on lines +60 to +61
Copy link

Choose a reason for hiding this comment

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

👀 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 False

which simplifies to

        return len(self.store) == 0

We could also remember that empty lists are falsy, and write this as

        return not self.store



def heap_up(self, index):
Expand All @@ -57,18 +68,40 @@ def heap_up(self, index):
property is reestablished.

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

Choose a reason for hiding this comment

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

👀 This function is the main source of time and space complexity for add, so refer back to that note.

"""
pass
if index == 0:
return index
Comment on lines +74 to +75
Copy link

Choose a reason for hiding this comment

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

👀 When we reach the top, we do want to stop, but we don't need to return a particular value, so

        if index == 0:
                return

alone would be sufficient.


parent_node = (index - 1) // 2
current_size = self.store
if current_size[parent_node].key > current_size[index].key:
self.swap(parent_node, index)
self.heap_up(parent_node)

def heap_down(self, index):
Copy link

Choose a reason for hiding this comment

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

✨ Nice set of conditionals to narrow in on where to swap, and then only perform the swap and continue the heapification if the key of the candidate requires it.

Though not prompted, like heap_up, heap_down is also O(log n) in both time and space complexity. The worst case for re-heapifying is if the new root need to move back down to a leaf, and so the stack growth will be the height of the heap, which is log n. If we implemented this instead with an iterative approach, the space complexity would instead be O(1).

""" 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
current_size = self.store
Copy link

Choose a reason for hiding this comment

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

👀 To me current_size sounds more like a number than a list of the data comprising the heap. When making an alias to an internal field (which allows us to avoid writing self so frequently), it's pretty common to use the same name as the aliased field for clarity.

        store = self.store

left_c = index * 2 + 1
right_c= index * 2 + 2

if left_c < len(self.store):
Copy link

Choose a reason for hiding this comment

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

👀 You made your alias to self.store, so be sure to use it here for consistency. The biggest concern around consistency is that when we do things inconsistently, it makes the person reading the code stop and wonder whether the inconsistency is meaningful, or just an oversight.

        if left_c < len(current_size):

if right_c < len(current_size):
if current_size[left_c].key < current_size[right_c].key:
less = left_c
else:
less = right_c
else:
less = left_c

if current_size[index].key > current_size[less].key:
self.swap(index, less)
self.heap_down(less)


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