feat: use smart pointer in binary search tree #2741
Conversation
1. Memory Management: Used std::unique_ptr for automatic memory management, reducing the risk of memory leaks. 2. STL Utilization: Replaced the custom queue implementation with std::queue, simplifying breadth-first traversal. 3. Code Structure: Organized the code into a BinaryTree class, encapsulating the tree functionality and providing clearer interfaces for operations. 4. Redundancy Reduction: Combined logic for finding and removing nodes to reduce redundancy, improving readability and maintainability. 5. Error Handling: Added checks to handle cases where nodes may not be present during removal and traversal.
arjunjain8887
requested review from
Panquesito7 and
realstealthninja
as code owners
| #include <memory> | ||
| #include <vector> | ||
| #include <queue> | ||
|
|
There was a problem hiding this comment.
should be inside the datastructures namespace
| */ | ||
| #include <iostream> | ||
| #include <memory> | ||
| #include <vector> |
There was a problem hiding this comment.
headers should be documented
|
|
||
| private: | ||
| // Helper function for insertion | ||
| void insertHelper(Node* node, int x) { |
There was a problem hiding this comment.
this could be inlined
| root = std::make_unique<Node>(x); // Create root if it doesn't exist | ||
| } else { | ||
| Insert(n->left, x); | ||
| insertHelper(root.get(), x); // Helper function for recursive insertion |
There was a problem hiding this comment.
should probably try to avoid recursion
|
|
||
| // Remove a value from the tree | ||
| void remove(int x) { | ||
| removeHelper(root, x); // Helper function to manage removal |
There was a problem hiding this comment.
instead of removeHelper why not _remove()?
| } | ||
|
|
||
| // Postorder traversal | ||
| void postOrderTraversal() const { |
There was a problem hiding this comment.
please use snake_case
| int main() { | ||
| queue.front = 0; | ||
| queue.rear = 0; | ||
| BinaryTree tree; |
There was a problem hiding this comment.
tests should be in a separate function called tests
|
Your code doesn't compile. The above review is about the minor issues. The major issue is the improper use of templated functions |
realstealthninja
added
automated tests are failing
realstealthninja
changed the title
|
This pull request has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
github-actions
bot
added
the
stale
|
Please ping one of the maintainers once you commit the changes requested or make improvements on the code. If this is not the case and you need some help, feel free to ask for help in our Gitter channel or our Discord server. Thank you for your contributions! |
Memory Management: Used std::unique_ptr for automatic memory management, reducing the risk of memory leaks.
STL Utilization: Replaced the custom queue implementation with std::queue, simplifying breadth-first traversal.
Code Structure: Organized the code into a BinaryTree class, encapsulating the tree functionality and providing clearer interfaces for operations.
Redundancy Reduction: Combined logic for finding and removing nodes to reduce redundancy, improving readability and maintainability.
Error Handling: Added checks to handle cases where nodes may not be present during removal and traversal.
Description of Change
Checklist
Notes: