Master #39
Master #39
Conversation
|
Grabbing this to grade! |
There was a problem hiding this comment.
Good work!
I left a few corrections on your time and space complexity calculations, a discussion about some performance issues with your solution for adding nodes, and a comment on a unneeded import statement, but overall this is a solid implementation so I'm fine giving it a Green.
| @@ -1,3 +1,6 @@ | |||
| from turtle import right | |||
There was a problem hiding this comment.
Looks like this didn't get used, so it can be deleted.
| else: | ||
| node.right = self.add_helper(node.right, key, value) | ||
|
|
||
| return node |
There was a problem hiding this comment.
This implementation of add_helper works, but definitely does a bit of extra work in setting nodes. Essentially, every node on the path to where the new node ends up will have either its left or right node "updated", though most of these will be updated to the exact same node. The only one that gets a meaningful update is whenever the bottom of the tree is reached and gets its left/right node updated to the new node. These updates happen in lines 26 and 27.
This probably will not be too bad of a performance hit, but there is a way that you can do this and just update only the node you need.
But this overall fine enough!
| @@ -14,35 +17,96 @@ class Tree: | |||
| def __init__(self): | |||
| self.root = None | |||
|
|
|||
| # Time Complexity: | |||
| # Space Complexity: | |||
| # Time Complexity: O(1) | |||
There was a problem hiding this comment.
Unfortunately as nodes are added to the tree, new nodes will have to be added deeper and deeper, meaning that this doesn't run in the constant time that O(1) implies. Assuming a balanced tree, the complexity will be O(log(n)).
For example, if there are 7 nodes in a perfectly balanced tree, there would be 1 on top, 2 on the next, and 4 on the next. To add an 8th node will require recursing 3 times to add the new one on the new fourth level. And log_2(8) is 3, since 2^3 is 8.
|
|
||
|
|
||
|
|
||
| # Time Complexity: O(N) |
There was a problem hiding this comment.
For much of the same reason as I explained above in my comment about adding a node, finding a node takes O(log(n)) time assuming a balanced tree.
For example, with 7 items in a perfectly balanced tree, searching for one of the values in the nodes on the bottom will mean iterating 3 times. The exact number is actually ceiling(log_2(n)) + 1, where ceiling rounds up the fractional part to the next number, which keeps this at O(log(n)). (Not that we expected you to come up with this number, I just wanted to explain!)
There was a problem hiding this comment.
I guess as was said in Learn, unbalanced trees do trend to O(n), but I think we can assume balanced trees ideally.
Either way, not going to take points off.
| # Time Complexity: | ||
| # Space Complexity: | ||
| # Time Complexity: O(1) | ||
| # Space Complexity: O(N) |
There was a problem hiding this comment.
Likewise, for a recursive implementation like this, the space used will be O(log(n)) because each recursion requires a bit of space on the stack as it recurses down.
| # Time Complexity: | ||
| # Space Complexity: | ||
| # Time Complexity: O(1) | ||
| # Space Complexity:O(1) |
There was a problem hiding this comment.
Like adding a node, you have to account for the time and space you recurse down the tree for both of these being O(log(n))
There was a problem hiding this comment.
Sorry, as I've done some of the other grading, I think the right calculation to give here is O(n) for time and O(1) for space. Because every node is being hit, our time complexity is O(n), but since we are not using more space, the space complexity is indeed O(1).
Forgive me for the conclusion, I was going off our example solution but I think I'm realizing there's a mistake in it.
No description provided.