2

This code is modified from a Stanford resource located here.

I started with this after some modifying to C++ conventions:

#include <iostream>
#include <string>

using namespace std;

template < typename A, typename B > class binary_tree
{
    public:

    class node 
    {
            public:
            A key;
        B value;
            node* left;
            node* right;
    };

    typedef node* nodePointer;

    nodePointer HP;

    nodePointer newNode( A key, B value ) 
    {
            nodePointer NP = new( node );
            NP->key = key;
                NP->value = value;
            NP->left = NULL;
            NP->right = NULL;
                return NP;
    }

    void insert( A key, B value )
    {
        insertI( this->HP, key, value );
    }

    nodePointer insertI( nodePointer NP, A key, B value ) 
    {
        if ( NP == NULL ) 
        {
            return newNode( key, value );
        }
        else 
        {
            if (key < NP->key) 
            {
                NP->left = insertI( NP->left, key, value );
            }
            else 
            {      
                NP->right = insertI( NP->right, key, value );
            }
            return NP;
        }
    }

    binary_tree(A key, B value)
    { 
        this->HP = insertI(NULL, key, value );
    }
};

To eliminate passing by value the key/value pair for each activation frame I updated the insertI() parameters to pass by const reference like this:

nodePointer insertI( nodePointer NP, const A& key, const B& value ) 
{
    if ( NP == NULL ) 
    {
        return newNode( key, value );
    }
    else 
    {
        if (key < NP->key) 
        {
            NP->left = insert( NP->left, key, value );
        }
        else 
        {      
            NP->right = insert( NP->right, key, value );
        }
        return NP;
    }
}

Right now I can only define the function inline b.c. I'm using an online compiler. But what is faster - inline, or defined outside of the class?

Also, this is just a stub, obviously the class over all needs more work. This question just regards the insertI() function.

Is there anything else I can do to make the insert more efficient?

Links

(http://en.wikipedia.org/wiki/Binary_search_tree#Searching)

(http://en.wikipedia.org/wiki/Binary_search_algorithm)

Another Approach

Tail Recursion

Notes

This operation requires O(log n) time in the average case, but needs O(n) time in the worst case, when the unbalanced tree resembles a linked list (degenerate tree).

Related

Improve this question
3
  • 1
    This function is not tail-recursive; tail-recursion optimization won't help you. Tail-recursive functions are those that recurse on a tail-call, where a tail-call is of the form "return foo(bar);" you are doing additional work (assignment) after making your recursive calls, which is a different situation. Also, why so worried about tree insertion efficiency? Seems like premature optimization. Commented Feb 29, 2012 at 22:13
  • Recursion, if that is your problem, can be eliminated by using a user managed stack (array) for the path not taken, and a loop. This can improve performance in some cases an is most useful if you know the maximum depth of your tree. Look at how qsort is implemented in glibc, for example. Commented Feb 29, 2012 at 22:29
  • Actually I can rewrite it in tail-recursive form with out the assignments and make NP a reference...from there I can add in balancing...I'll update once complete. Commented Mar 1, 2012 at 0:49

2 Answers 2

Reset to default
2

But what is faster - inline, or defined outside of the class?

Defining methods in the class declaration can be worth it for small methods. For larger functions the jump is negligible.

Is there anything else I can do to make the insert more efficient?

I wouldn't use recursion in places where a simple loop would suffice. Compilers are pretty good at optimizing recursion, but the same is true for loop optimization.

You can balance the tree to improve runtime. For example:
http://en.wikipedia.org/wiki/Red–black_tree

Also, this occurs to me:

void insert(const A& key, const B& value) {
  node** cur = &HP;
  while (*cur != NULL) {
    cur = &(key < (*cur)->key ? (*cur)->left : (*cur)->right);
  }
  *cur = new node(key, value);
}

Traversing the pointers themselves rather than the nodes that contain them.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

2

Does it really make sense to improve the efficiency of the insert method if you do not implement the tree-balancing part? If you insert items such as 1>2>3>4>... you will always end-up in O(n) operation to insert the next item.

I'd suggest to first auto-balance your tree and then there will be matter to improvement. And if recursion is an issue you can easily remove it with something such as:

nodePointer newOne = newNode(...);

if(NP== NULL)
    return newOne;

while(NP != NULL) {
    if(key < NP->key) {
        if(NP->left == NULL) {NP->left = newOne; return;} // finally inserted
        else                 NP = NP->left;

    } else {
        if(NP->right== NULL) {NP->right= newOne; return;} // finally inserted
        else                 NP = NP->right;
    }
}

// Should never reach this line
Improve this answer

1 Comment

<i>Does it really make sense to improve the efficiency of the insert method if you do not implement the tree-balancing part?</i> What will be the insert complexity if the data is uniformly distributed?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.