Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdded Remove Method To Red Black Tree #175
Conversation
|
Ok I got the google java style guide. Sorry for the messiness, I just learned how to enable google java style guide. |
| public void testNumberDoesntExist() { | ||
| assertFalse(tree.delete(0)); | ||
| } | ||
|
|
||
| /* | ||
| @Test | ||
| public void randomRemoveTests() { |
This comment has been minimized.
This comment has been minimized.
williamfiset
May 10, 2020
Owner
Can you uncomment the randomTemoveTests method? You may need to adapt it to work, but it should catch any bugs
This comment has been minimized.
This comment has been minimized.
nishantc1527
May 10, 2020
Author
Contributor
Oh, sorry about that. By the way (this is not a hate comment, just a suggestion), it would much easier if instead of null checks, you used a constant field called NIL or something which is a node with the color of black, the left child is NIL, the right child is NIL, and the parent is NIL. So, instead of constantly doing if(node == null || node.color == BLACK) you can do if(node.color == BLACK). Like I said, just a suggestion. I didn't change it because you were the original author.
|
Yeah I need to add quite a few null checks. My original source code didn't account for this. |
|
I need to redo some stuff. I'll come back in a bit. |
| } | ||
|
|
||
| @Test | ||
| public void testTreeHeight() { |
This comment has been minimized.
This comment has been minimized.
williamfiset
May 11, 2020
Owner
Don't feel the need to include the tree height test and method if it's causing a headache. This is more important for AVL trees than for RB trees. Feel free to remove if you want.
This comment has been minimized.
This comment has been minimized.
nishantc1527
May 11, 2020
Author
Contributor
Oh thanks! But it wasn't that, there were some bugs in my actual delete method, and since I made some stuff really messy, I'm thinking of redoing it. It won't be much though, since all I need to do is copy and paste my source code.
|
If it's fine with you, I'm going to change most of the stuff by adding the NIL change I was talking about earlier. When I push it, you can tell me to delete it if you want. |
|
Ok, so when adding the NIL node, I had to change a lot of stuff in the original insert method. If you want, I'll try my best to make it look as similar as possible to what it looked like before. However, working without the NIL node is like not having a pointer to the parent node. It's possible, but much harder. |
|
The NIL node is fine, I can see how it would be a pain to reach into nested references while constantly checking for null. Thanks for reworking this and making the tests pass :) |
nishantc1527 commentedMay 9, 2020
•
edited
resolves #80