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

Reid C16 Spruce #28

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

Reid C16 Spruce #28

wants to merge 1 commit into from

Conversation

reidhowdy
Copy link

Heaps Practice

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How is a Heap different from a Binary Search Tree?
Could you build a heap with linked nodes?
Why is adding a node to a heap an O(log n) operation?
Were the heap_up & heap_down methods useful? Why?

@anselrognlie anselrognlie self-requested a review July 12, 2022 22:10
Copy link

@anselrognlie anselrognlie left a comment

Choose a reason for hiding this comment

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

✨💫 Nice job on your implementation, Reid. I left some comments below.

You left the comprehension questions blank, and a few of the complexity questions are incomplete, so I'm grading this as a yellow for now. But please feel free to resubmit if you would like a green score!

🟡

Comment on lines +25 to 26
Time Complexity: O(log n)
Space Complexity: ?

Choose a reason for hiding this comment

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

👀 Your time complexity looks good, but what about the space complexity?

Space Complexity: ?
"""
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

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

Comment on lines +47 to 48
Time Complexity: O(log n)
Space Complexity: ?

Choose a reason for hiding this comment

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

👀 Again, time complexity looks good, but what about the space complexity?

Comment on lines 75 to 76
Time complexity: ?
Space complexity: ?

Choose a reason for hiding this comment

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

👀 What are the time and space complexities for empty?

Comment on lines 88 to 89
Time complexity: ?
Space complexity: ?

Choose a reason for hiding this comment

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

👀 What are the time and space complexities for heap_up?

Comment on lines +78 to +79
if not self.store:
return True

Choose a reason for hiding this comment

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

👀 An empty list is falsy, 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 not self.store:
            return True
        else:
            return False

which simplifies to

        return not self.store

#we do this recursively
#so we check until everything is in the right place



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, consider what the time and space complexity is for heap_down. Does it differ from heap_up at all? What impact does the recursive call have on the complexity? Could this be improved?

Comment on lines 4 to 5
Time Complexity: ?
Space Complexity: ?

Choose a reason for hiding this comment

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

👀 What are the time and space complexities for heap_sort?

Your approach looks good. To heap sort, we build a heap from the data to be sorted, then we remove the values from the heap, which gives them back to us in order. What does this mean for the time and space complexity?

Comment on lines +19 to +21
nums[index] = heap.remove()
index += 1

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

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