Creating a better BST

I have created a BST like many others. I am curious to if there is a better way to write it. It seems pretty straight forward, but I feel like it can be improved on a bit.

```
//MRBST.h

#ifndef MRBST_H
#define MRBST_H
#include

template
class MRBST {

private:
struct treeNode
{
treeNode* left;
treeNode* right;
int data;

};
treeNode* root;
int count;

public:
void push(const T &);
MRBST();
bool search(const T &);
void PrintPreorder();
~MRBST();
bool empty() const;
int numberOfNodes() const;
void preorder(treeNode* pre);

};

template
MRBST::MRBST()
{
root = NULL;
count = 0;
}

template
void MRBST::push(const T & n)
{
treeNode* d = new treeNode;
treeNode* parent;
d->data = n;
d->left = NULL;
d->right = NULL;
parent = NULL;
if(empty())
{
root = d;
count++;
}
else
{
treeNode* current;
current = root;

while(current != NULL)
{
parent = current;
if(d->data >= current->data)
{
current = current->right;
}
else
{
current = current->left;
}
if(d->data data)
{
parent->left = d;
}
else
{
parent->right = d;
}
}
count++;
}
}

template
int MRBST::numberOfNodes() const
{
return count;
}

template
bool MRBST::empty() const
{
return root == NULL;
}

template
void MRBST::PrintPreorder()
{
preorder(root);
}

template
void MRBST::preorder(treeNode* pre)
{
if(pre != NULL)
{
std::cout data left)
{
preorder(pre->left);
}
if(pre->right)
{
preorder(pre->right);
}
}
}

template
MRBST::~MRBST()
{
delete ro

There are several improvements that can be done, starting from the class declaration:

-
The class name is not very descriptive. MRBST could be an initialism for many things. BinarySearchTree is much better. Don't be afraid of using long names if they convey information more clearly.

-
One thing doesn't make sense here: The class is a template class, but the data field of the treeNode is an int. If you are only storing ints, then the class doesn't have to be a template. You should however replace that int with T and make it generic.

-
The naming convention of treeNode is a little unusual for types. This kind of camelCase notation is usually applied to variables. Types themselves are more commonly declared using CamelCase, fist letter uppercase.

-
PrintPreorder() breaks your naming convention for methods. printPreorder() would be more consistent.

-
Ordering of class data and member methods: I find more adequate placing public methods and data first, followed by protected methods/data and lastly anything private. This is convenient for readers of the class declaration, since they will be interested in the public interface most of the time.

-
Order of the public fields: Place constructors first, followed by assignment/move operators, then the destructor and the rest after those. The fist thing you want to inspect when reading a class declaration is how the constructor(s), destructor and assignment are implemented, because that gives you hints about how the class lifetime is managed.

-
Unnamed function parameters in push() and search(): You should always name function parameters in the header declaration because that's a form of code documentation.

-
Remove excessive blank lines from the header file. There are some unexpected blank lines in there that serve no purpose. It looks untidy.

More aspect to improve in the implementation:

-
Please don't use NULL in C++ code. Your compiler should be C++11 ready by now, so use nullptr.

-
PrintPreorder() and preorder() should also be const methods since they don't mutate member state. Also the treeNode pointer that preorder() takes should be a const pointer. E.g.: const treeNode* pre.

-
Currently, your class leaks memory. You are only deleting the root of the tree. All leaf nodes will leak. You need to recursively delete all nodes before deleting the root. The process is basically the same as how you did to print the tree.

There are probably more details that can be improved, as well as architectural improvements. Other reviews might want to expand on those.

StackExchange Code Review Q#68133, answer score: 7