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

Julia Kingrey #26

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Julia Kingrey #26

wants to merge 6 commits into from

Conversation

Kalakalot
Copy link

Heaps Practice

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How is a Heap different from a Binary Search Tree? A heap doesn't have the same child requirements as a binary search tree. A binary search tree requires each left child to have a smaller key value than the parent, and the right child to have a higher key value than the parent, while a min heap just requires that each child be larger than the parent (max heaps have children that are smaller than the parent). Also, binary search trees can be unbalanced, while a valid heap must have every level but the last filled, and that level must be filled from left to right.
Could you build a heap with linked nodes? Yes but implementing with an array is usually easier.
Why is adding a node to a heap an O(log n) operation? Because we only need to make one comparison/swap for (at most) each level of the tree.
Were the heap_up & heap_down methods useful? Why? Yes, creating a recursive heap_up / heap_down method helped separate the two parts of the add and remove functions: the code in my add method just swapped the first and last elements and removed the last, while the heap_up helper took care of the heap reordering. Separation of concerns is good design!

Copy link

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

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

Nice work, you hit most of the learning goals here. Some issues with heap_down, which is a tough method. Otherwise things look pretty good. Take a look at my comments and let me know any questions you have.

Comment on lines +2 to +5
# Time Complexity: n log n -- n to put elements in heap, log n to sort,
# and n log n put them back in the array
# Space Complexity: O(n)
def heapsort(list)

Choose a reason for hiding this comment

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

👍 Well done.

Comment on lines +18 to 21
# Time Complexity: usually O(1) because unlike adding to the front, appending /
# adding to the end doesn't require reordering the whole array
# Space Complexity: O(1)
def add(key, value = key)

Choose a reason for hiding this comment

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

The time complexity is O(log n) because the heap_up method is O(log n). Because it's recursive, the space complexity is also O(log n)

Otherwise this works.

Comment on lines +30 to +34
# Time Complexity: O(1) because we are doing a swap and deleting at the end
# (would be O of n if we were deleting from the front)
# Space Complexity: O(1) because we're not creating any new spaces in memory

def remove

Choose a reason for hiding this comment

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

This works but has similar time/space complexity issues to add.

def empty?
raise NotImplementedError, "Method not implemented yet..."
@store.length == 0 ? true : false

Choose a reason for hiding this comment

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

Suggested change
@store.length == 0 ? true : false
return @store.length == 0

Comment on lines +75 to 78
# Time complexity: O(1) -- I *think* this code will stop running once
# count gets higher than 0 but I'm not certain
# Space complexity: O(1)
def empty?

Choose a reason for hiding this comment

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

👍

Comment on lines +87 to 89
# Time complexity: O(log n) because we're swapping max once per tree level
# Space complexity: O(log n)
def heap_up(index)

Choose a reason for hiding this comment

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

👍

# stop if we're at the last leaf in the tree
return if @store[left_child_index].nil? && @store[right_child_index].nil?
# stop if both children are greater than or equal to parent
return if @store[left_child_index].key >= @store[index].key && @store

Choose a reason for hiding this comment

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

What if the right index child is not nil and it's less than @store[index]??? Looks like you missed an edge-case.

# if parent is greater than left child, swap and look at next level --
# need @store[index] conditional to stop code running after we've reached
# the bottom of the heap
if @store[index] && @store[index].key > @store[left_child_index].key

Choose a reason for hiding this comment

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

You also need to figure out which is smaller the left child or right child. Otherwise you'll do some unnecessary heap_downs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants