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

Branches - Julia Bouvier #37

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

Conversation

juliabouv
Copy link

No description provided.

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.

Really nice work, you hit most of the learning goals here. Very elegantly written code. The only issue is with height, as it's not quite working. Check out my comments and let me know any questions you have.

Comment on lines +19 to 21
# Time Complexity: log(n)
# Space Complexity: log(n)
def add(key, value)

Choose a reason for hiding this comment

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

👍

Comment on lines +43 to 45
# Time Complexity: log(n)
# Space Complexity: log(n)
def find(key)

Choose a reason for hiding this comment

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

👍

end
end

def delete(key)

Choose a reason for hiding this comment

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

Just noting this isn't working.


def remove(node)
if node.left.nil? && node.right.nil?
node = nil

Choose a reason for hiding this comment

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

This only changes the local variable (node) reference.

Comment on lines +114 to 116
# Time Complexity: O(n)
# Space Complexity: O(n)
def inorder

Choose a reason for hiding this comment

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

👍

Comment on lines +130 to 132
# Time Complexity: O(n)
# Space Complexity: O(n)
def preorder

Choose a reason for hiding this comment

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

👍

Comment on lines +144 to 146
# Time Complexity: O(n)
# Space Complexity: O(n)
def postorder

Choose a reason for hiding this comment

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

👍

Comment on lines +158 to 160
# Time Complexity: O(n)
# Space Complexity: O(log n)
def height

Choose a reason for hiding this comment

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

This method isn't actually working. See my note below.

Think about this:

If the node is nil then return 0
If the node is not 0, find the height of the left and the right.

Return the height of the bigger subtree and add one (for the current node).

lib/tree.rb Outdated
end

height_helper(node.left, left + 1, right)
height_helper(node.right, left, right + 1)

Choose a reason for hiding this comment

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

This is only returning the right side's height!

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