1

I am trying to create a simple Binary Tree, then add several integers to the tree and finally print out the height of the tree. Although I seem to be getting a stack overflow exception when calling the BinaryTree height() method. Can anyone tell me why? Please advise if I have implemented something incorrectly as this is my first time working with Binary Trees (I have looked over a few tutorials and came up with this code)

TreeNode.cpp

#include "TreeNode.h"


TreeNode::TreeNode(int theData)
{
    theData = value;
}

BinaryTree.cpp

#include "BinaryTree.h"

BinaryTree::BinaryTree()
{
    root = NULL;
}

void BinaryTree::add(int data)
{
    if (root != NULL) {
        add(root,data);
    }
    else {
        root = new TreeNode(data);
        root->value = data;
        root->left = NULL;
        root->right = NULL;
    }
}

int BinaryTree::height()
{
    return height(root);
}



void BinaryTree::add(TreeNode * toAdd, int key)
{
    if (key < toAdd->value) {
        if (toAdd->left != NULL) {
            add(toAdd->left, key);
        }
        else {
            toAdd->left = new TreeNode(key);
            toAdd->left->value = key;
            toAdd->left->left = NULL;
            toAdd->left->right = NULL;
        }
    }
    else {
        if (toAdd->right != NULL) {
            add(toAdd->right, key);
        }
        else {
            toAdd->right = new TreeNode(key);
            toAdd->right->value = key;
            toAdd->right->left = NULL;
            toAdd->right->right = NULL;
        }
    }

}

int BinaryTree::height(TreeNode *node)
{
    if (root == NULL) {
        return 0;
    }
    else {

        int leftSide = height(root->left);
        int rightSide = height(root->right);
        int total = leftSide + rightSide;
        return total;
    }

}

Main.cpp

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

int main()
{
    BinaryTree *tree = new BinaryTree();

    tree->add(10);
    tree->add(12);
    tree->add(14);
    tree->add(15);
    tree->add(123);
    tree->add(14);
    tree->add(12);
    tree->add(15);

    tree->height();


    return 0;
}
craig2020
  • 321
  • 1
  • 6
  • 12

1 Answers1

1

height references the wrong variable. Since it looks at root, and completely ignores the parameter node, every call to height will be the same and the recursion, once started, will never end.

Replace all references to root with node in height.

if (node == nullptr) 
    return 0;
int leftside = height(node->left);
int rightside = height(node->right);

And a few unrelated things:

height could be declared const, or made a static member of BinaryTree, or moved into the Node class.

You have a constructor for TreeNode, which should handle initializing all of the members of the class. When you allocate or add a node you should just have the one line wherever = new TreeNode(key);. (And what's the difference between theData and value? Do you need to distinct fields for these?)

The BinaryTree *tree in main can just be an object, and does not need to be dynamically allocated (BinaryTree tree;, then change all the tree-> to tree.).

1201ProgramAlarm
  • 32,384
  • 7
  • 42
  • 56
  • Thanks a lot for the detailed explanation. I will make the changes now and hopefully get it working – craig2020 Feb 08 '20 at 18:34
  • I made the changes to the height() method; changed root to node etc. so I no longer get a stack overflow error, although the method keeps returning 0 in console. Does the other implementation in height() look correct to you? – craig2020 Feb 08 '20 at 20:58
  • 1
    @craig2020 The base case (root == NULL) returns 0, and you never add anything to it when you call it recursively, so it can't return anything other than 0. – 1201ProgramAlarm Feb 08 '20 at 21:56