I'm wondering if calling a private function from a public function to achieve a cleaner syntax could cause any type of problems.

#include<iostream>

class tree{
    private:
    struct node {
        int data;
        int counter = 1;
        node* left;
        node* right;
    };
    
    node* getnewnode(int x) {
        node* temp = new node();
        temp -> data = x;
        temp -> left = NULL;
        temp -> right = NULL;
        return temp;
    }

    node* recursiveinsert(int x, node* rootPtr) {// recursively insert a new node
        if(rootPtr==NULL) { // if the tree is empty, append the new node
            rootPtr = getnewnode(x);
        } 
        else if(x <= rootPtr->data){ // if x is lesser than the node value, make a recursive call with the left subtree as root
            rootPtr -> left = recursiveinsert(x, rootPtr -> left);
        }
        else { //if x is greater than the node value, make a recursive call with the right subtree as root
            rootPtr -> right = recursiveinsert(x, rootPtr -> right);
        }
        return rootPtr;
    }

    public:

    //store address of root node
    node* root = NULL;

    void insert(int x) {
        root = recursiveinsert(x, root);
    }
};

Using the tree class, instead of calling in main:

int main(){
    tree t;
    t.root = t.recursiveinsert(10, t.root);
}

I thought it'd be cleaner to call this instead:

int main(){
    tree t;
    t.insert(10);
}

Is this a good coding practice?

CodePudding user response：

For starters the data member

node* root = NULL;

shall not be public.

Secondly the function recursiveinsert should be declared at least as a static member function.

It is better to declare and define it like

static void recursiveinsert( node * &rootPtr, int x ) 
{
    if ( rootPtr == nullptr ) 
    {
        rootPtr = getnewnode(x);
    } 
    else if ( x < rootPtr->data )
    {
        recursiveinsert( rootPtr -> left, x );
    }
    else 
    {
        recursiveinsert( rootPtr -> right, x );
    }
}

There is nothing wrong to call a private member function from a public member function. For example it is not rare when a constructor of a class calls a private member function in its mem-initializer list.

The function getnewnode can look simpler. For example

node* getnewnode( int x)  
{
    return new node { x, 1, nullptr, nullptr };
}

CodePudding user response：

You would probably also want to hide the 'root' member so that setting it directly wouldn't even be possible. Then have an accessor like:

const node * GetRoot () { return node; }