Search code examples
c++classbinary-search-treeaccess-controlfunction-call

Is calling a private function from a public function good coding practice?


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?


Solution

  • 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 };
    }