Skip to content
Open

BST #26

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
223 changes: 76 additions & 147 deletions data_structures-II/06-BST/bst.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,199 +12,128 @@
#ifndef BST_H
#define BST_H

//*****************************************************************************************************

#include "node.h"
#include <iostream>

//*****************************************************************************************************
using namespace std;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid using namespace std; in header files.

This directive pollutes the global namespace for every file that includes this header, potentially causing name collisions and making code harder to maintain.

🔧 Proposed fix
-using namespace std;

Then qualify standard library types explicitly (e.g., std::ostream, std::cout, std::endl).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@data_structures-II/06-BST/bst.h` at line 17, Remove the global "using
namespace std;" declaration from the header (the standalone using namespace std;
line) and update all STD symbols declared or referenced in this header to be
fully qualified (e.g., change ostream, string, cout, endl, etc. to std::ostream,
std::string, std::cout, std::endl); ensure any function signatures, class
members, or friend operators in bst.h (e.g., operator<< or any public APIs) use
the std:: prefix so including this header won't pollute the global namespace.


template <typename T>
class BST {

private:
Node<T> *root;

int _max(int a, int b) const;
void _deleteTree(Node<T> *&root);
void _insert(Node<T> *&root, const T &item);
void _inOrder(Node<T> *root, std::ostream &out) const;
void _preOrder(Node<T> *root, std::ostream &out) const;
void _postOrder(Node<T> *root, std::ostream &out) const;
T *_search(Node<T> *root, const T &item) const;
int _height(Node<T> *root) const;
// Node<T> *_findMin(Node<T> *root);
Node<T>* root;

public:
BST();
~BST();
void destroy();
void insert(const T &item);
void inOrder(std::ostream &out = std::cout) const;
void preOrder(std::ostream &out = std::cout) const;
void postOrder(std::ostream &out = std::cout) const;
T *search(const T &item) const;
int height() const;
};
Node<T>* insert(Node<T>* node, const T& value) {

//*****************************************************************************************************
if(node == nullptr)
return new Node<T>(value);

template <typename T>
BST<T>::BST() {
root = nullptr;
}

//*****************************************************************************************************
if(value < node->data)
node->left = insert(node->left,value);

template <typename T>
BST<T>::~BST() {
_deleteTree(root);
}
else if(node->data < value)
node->right = insert(node->right,value);

//*****************************************************************************************************
return node;
}

template <typename T>
int BST<T>::_max(int a, int b) const {
return (a > b) ? a : b;
}
Node<T>* searchNode(Node<T>* node,const T& key) const {

//*****************************************************************************************************
if(node == nullptr)
return nullptr;

template <typename T>
void BST<T>::destroy() {
_deleteTree(root);
}
if(key < node->data)
return searchNode(node->left,key);

//*****************************************************************************************************
if(node->data < key)
return searchNode(node->right,key);

template <typename T>
void BST<T>::_deleteTree(Node<T> *&root) {
if (root != nullptr) {
_deleteTree(root->left);
_deleteTree(root->right);
delete root;
root = nullptr;
return node;
}
}

//*****************************************************************************************************
void inOrder(Node<T>* node,ostream& out) const {

template <typename T>
void BST<T>::insert(const T &item) {
_insert(root, item);
}
if(node==nullptr) return;

//*****************************************************************************************************
inOrder(node->left,out);
out<<node->data<<endl;
inOrder(node->right,out);
}

template <typename T>
void BST<T>::_insert(Node<T> *&root, const T &item) {
if (root == nullptr)
root = new Node<T>(item);
else if (item < root->value)
_insert(root->left, item);
else if (item > root->value)
_insert(root->right, item);
else
std::cerr << "\nError: duplicate value\n";
}
void preOrder(Node<T>* node) const {

//*****************************************************************************************************
if(node==nullptr) return;

template <typename T>
void BST<T>::inOrder(std::ostream &out) const {
_inOrder(root, out);
}
cout<<node->data<<endl;
preOrder(node->left);
preOrder(node->right);
}

//*****************************************************************************************************
void postOrder(Node<T>* node) const {

template <typename T>
void BST<T>::_inOrder(Node<T> *root, std::ostream &out) const {
if (root != nullptr) {
_inOrder(root->left, out);
out << root->value << std::endl;
_inOrder(root->right, out);
if(node==nullptr) return;

postOrder(node->left);
postOrder(node->right);
cout<<node->data<<endl;
}
}

//*****************************************************************************************************
int height(Node<T>* node) const {

template <typename T>
void BST<T>::preOrder(std::ostream &out) const {
_preOrder(root, out);
}
if(node==nullptr)
return 0;

//*****************************************************************************************************
int leftHeight = height(node->left);
int rightHeight = height(node->right);

template <typename T>
void BST<T>::_preOrder(Node<T> *root, std::ostream &out) const {
if (root != nullptr) {
out << root->value << std::endl;
_preOrder(root->left, out);
_preOrder(root->right, out);
if(leftHeight > rightHeight)
return leftHeight+1;
else
return rightHeight+1;
}
}

//*****************************************************************************************************

template <typename T>
void BST<T>::postOrder(std::ostream &out) const {
_postOrder(root, out);
}
public:

//*****************************************************************************************************
BST() {
root = nullptr;
}
Comment on lines +94 to +98
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Missing destructor causes memory leak.

The BST owns dynamically allocated Node<T> objects but has no destructor to free them. When a BST object goes out of scope (as shown in main.cpp at lines 32-51), all nodes remain allocated, causing a memory leak.

Additionally, this violates the Rule of Three/Five: if a class manages resources requiring custom destruction, it should also define (or delete) the copy constructor and copy assignment operator to prevent double-free or shallow-copy issues.

🐛 Proposed fix: Add destructor and delete tree helper
 private:
     Node<T>* root;
 
+    void deleteTree(Node<T>* node) {
+        if (node != nullptr) {
+            deleteTree(node->left);
+            deleteTree(node->right);
+            delete node;
+        }
+    }
+
     Node<T>* insert(Node<T>* node, const T& value) {
 public:
 
     BST() {
         root = nullptr;
     }
+
+    ~BST() {
+        deleteTree(root);
+    }
+
+    // Prevent shallow copies (or implement deep copy if needed)
+    BST(const BST&) = delete;
+    BST& operator=(const BST&) = delete;
 
     void insert(const T& value) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public:
//*****************************************************************************************************
BST() {
root = nullptr;
}
private:
Node<T>* root;
void deleteTree(Node<T>* node) {
if (node != nullptr) {
deleteTree(node->left);
deleteTree(node->right);
delete node;
}
}
Node<T>* insert(Node<T>* node, const T& value) {
// ... rest of implementation
public:
BST() {
root = nullptr;
}
~BST() {
deleteTree(root);
}
// Prevent shallow copies (or implement deep copy if needed)
BST(const BST&) = delete;
BST& operator=(const BST&) = delete;
void insert(const T& value) {
// ... rest of implementation
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@data_structures-II/06-BST/bst.h` around lines 94 - 98, BST currently lacks a
destructor and Rule-of-Three/Five safeguards, causing Node<T> heap leaks and
potential shallow-copy issues; implement a destructor ~BST that calls a
recursive helper (e.g., deleteTree(Node<T>*)) to delete all nodes starting from
root, and either define or delete the copy constructor and copy-assignment
operator (BST(const BST&), BST& operator=(const BST&)) to prevent shallow
copies; ensure deleteTree traverses left and right children before deleting the
node and set root to nullptr after cleanup.


template <typename T>
void BST<T>::_postOrder(Node<T> *root, std::ostream &out) const {
if (root != nullptr) {
_postOrder(root->left, out);
_postOrder(root->right, out);
out << root->value << std::endl;
void insert(const T& value) {
root = insert(root,value);
}
}

//*****************************************************************************************************
T* search(const T& key) const {

template <typename T>
T *BST<T>::search(const T &item) const {
return _search(root, item);
}
Node<T>* found = searchNode(root,key);

//*****************************************************************************************************
if(found==nullptr)
return nullptr;

template <typename T>
T *BST<T>::_search(Node<T> *root, const T &item) const {
T *result = nullptr;

if (root != nullptr) {
if (item > root->value)
result = _search(root->right, item);
else if (item < root->value)
result = _search(root->left, item);
else
result = new T(root->value);
return new T(found->data);
}
Comment on lines +104 to 112
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Memory leak risk: search() returns heap-allocated copy.

The search method allocates a new T object on the heap (line 111) and returns a raw pointer. The caller must manually delete the result, which is undocumented and error-prone. If the caller forgets to delete, memory leaks occur.

Consider returning a pointer to the existing node's data (const if appropriate), or return nullptr when not found:

🔧 Proposed fix: Return pointer to existing data
     T* search(const T& key) const {
 
         Node<T>* found = searchNode(root,key);
 
         if(found==nullptr)
             return nullptr;
 
-        return new T(found->data);
+        return &(found->data);
     }

Note: If mutability is a concern, consider returning const T* instead.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
T* search(const T& key) const {
template <typename T>
T *BST<T>::search(const T &item) const {
return _search(root, item);
}
Node<T>* found = searchNode(root,key);
//*****************************************************************************************************
if(found==nullptr)
return nullptr;
template <typename T>
T *BST<T>::_search(Node<T> *root, const T &item) const {
T *result = nullptr;
if (root != nullptr) {
if (item > root->value)
result = _search(root->right, item);
else if (item < root->value)
result = _search(root->left, item);
else
result = new T(root->value);
return new T(found->data);
}
const T* search(const T& key) const {
Node<T>* found = searchNode(root,key);
if(found==nullptr)
return nullptr;
return &(found->data);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@data_structures-II/06-BST/bst.h` around lines 104 - 112, The search method
currently allocates a new T and returns a raw pointer (search), causing
potential leaks; change search's return to return a pointer to the node's
existing data instead of allocating (e.g., return const T* or T* depending on
mutability), directly return found->data (or &found->data) and remove the
new/delete requirement, and update the caller usages and the declaration of
search (and any overloads) accordingly; keep using searchNode and Node<T> and
still return nullptr when not found.


return result;
}

//*****************************************************************************************************
void inOrder() const {
inOrder(root,cout);
}

template <typename T>
int BST<T>::height() const {
return _height(root);
}
void inOrder(ostream& out) const {
inOrder(root,out);
}

//*****************************************************************************************************
void preOrder() const {
preOrder(root);
}

template <typename T>
int BST<T>::_height(Node<T> *root) const {
int result = 0;
void postOrder() const {
postOrder(root);
}

if (root != nullptr)
result = 1 + _max(_height(root->left), _height(root->right));
int height() const {
return height(root);
}

return result;
}
};

#endif
//*****************************************************************************************************

// template <typename T>
Expand All @@ -222,4 +151,4 @@ int BST<T>::_height(Node<T> *root) const {

//*****************************************************************************************************

#endif
#endif
39 changes: 10 additions & 29 deletions data_structures-II/06-BST/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,17 @@
#ifndef NODE_H
#define NODE_H

//*****************************************************************************************************

template <typename T>
struct Node {
T value;
Node<T> *left;
Node<T> *right;

Node();
Node(const T &v, Node<T> *l = nullptr, Node<T> *r = nullptr);
T data;
Node<T>* left;
Node<T>* right;

Node(const T& value) {
data = value;
left = nullptr;
right = nullptr;
}
};

//*****************************************************************************************************

template <typename T>
Node<T>::Node() {
value = T(); // T() - default initialization (0 for numbers, empty string for strings, etc.)
left = nullptr;
right = nullptr;
}

//*****************************************************************************************************

template <typename T>
Node<T>::Node(const T &v, Node<T> *l, Node<T> *r) {
value = v;
left = l;
right = r;
}

//*****************************************************************************************************

#endif
#endif
Loading