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

Leaves - Mariya #30

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

Leaves - Mariya #30

wants to merge 1 commit into from

Conversation

M-Burr
Copy link

@M-Burr M-Burr commented Mar 22, 2020

Heaps Practice

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How is a Heap different from a Binary Search Tree? Binary Search Trees do not have the same child requirements as heaps. Heaps are typically complete trees, while BSTs can be unbalanced.
Could you build a heap with linked nodes? It's possible but arrays are typically used.
Why is adding a node to a heap an O(log n) operation? The depth of the tree is Log (n). Adding a node is Log (n) because you do at most, log(n) swaps.
Were the heap_up & heap_down methods useful? Why? They were helpful recursive helper methods that helped my code from becoming unnecessarily long and confusing.

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.

Overall nice work, you hit the major learning goals here. Well done. Take a look at my feedback and let me know if you have any questions. Most of the feedback is centered around BigO and extra recursive calls on heap_up and heap_down.

Comment on lines +4 to 6
# Time Complexity: O (n log (n))
# Space Complexity: O(n)
def heap_sort(list)

Choose a reason for hiding this comment

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

👍

Comment on lines +17 to 19
# Time Complexity: O(log(n))
# 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.

Since heap_up is recursive the space complexity is O(log n)

Comment on lines +28 to 30
# Time Complexity: O(log(n))
# Space Complexity: O(1)
def remove()

Choose a reason for hiding this comment

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

👍 However like add the space complexity is O(log n)

Comment on lines +61 to +65
if @store.length == 0
return true
else
return false
end

Choose a reason for hiding this comment

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

Suggested change
if @store.length == 0
return true
else
return false
end
return @store.length == 0

Comment on lines +58 to 60
# Time complexity: O(1)
# 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 +82 to +84
while parent > 0
return heap_up(parent)
end

Choose a reason for hiding this comment

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

Note you're making extra recursive calls here. You only want to heap_up if you made a swap, and since you're returning here you don't need a while loop.

Comment on lines +73 to 75
# Time complexity: O(log (n))
# Space complexity: O(1)
def heap_up(index)

Choose a reason for hiding this comment

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

Since this is a recursive method, it's going to be O(log n) space complexity.

swap(index, smallest_child)
end

return heap_down(smallest_child)

Choose a reason for hiding this comment

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

Similar to the heap_up method you're making extra recursive calls. You only need to continue to heap_down if you make a swap.

end


# This helper method takes an index and
# moves it up the heap if it's smaller
# than it's parent node.
def heap_down(index)

Choose a reason for hiding this comment

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

This works, but it has issues with extra swaps.

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