My binary tree program crashes at output

-1

I'm trying to code a binary tree, but it crashes. It keeps printing out the same numbers : left node(smaller number) and the root node. It repeats the program recursively until it crashes. I know my code has some other bugs but this is the main one to solve so here is my code:

#include <iostream>

using namespace std;

struct node {
    int value = 0;
    node* left = NULL;
    node* right = NULL;
};

node root;
void add(int x, node* curr)
{

    if (x < (*curr).value) {

        if ((*curr).left == NULL) {
            node next;
            next.value = x;
            (*curr).left = &next;
        }
        else {
            add(x, (*curr).left);
        }

        if (x > (*curr).value) {
            if ((*curr).right == NULL) {
                node next;
                next.value = x;
                (*curr).right = &next;
            }
            else {
                add(x, (*curr).right);
            }
        }
    }
}

void out(node ro)
{
    node lefta;
    node righta;
    if (ro.left != NULL) {
        lefta = *(ro.left);
        cout << lefta.value;
        out(lefta);
    }
    if (ro.right != NULL) {
        righta = *(ro.right);
        cout << " " << righta.value << endl;
        out(righta);
    }
}

int main()
{

    int n;
    cin >> n;
    int x;
    cin >> x;
    root.value = x;
    node* curr;
    curr = &root;
    for (int i = 1; i < n; i++) {
        cin >> x;
        add(x, curr);
    }

    out(root);

    return 0;
}

After the crash it returns me:

Process returned -1073741571 (0xC00000FD)
c++
tree
binary-tree
binary-search-tree
asked on Stack Overflow Jan 2, 2016 by CyberD • edited Feb 7, 2018 by Vadim Kotov

1 Answer

1

If you like to insert an new node into your list you have to allocate it. A local variable on stack goes out of scope when function returns. Anyway you have to adapt your function add for double linked list like this:

void add(int x, node *curr)
{
    // while x less than curr->value step left 
    while ( curr->left != NULL && x < curr->value )
        curr = curr->left;

    // while x greater than curr->next->value step right 
    while ( curr->right != NULL && x > curr->right->value )
        curr = curr->right;

    // x is less than curr->right->value (curr->right may be NULL)
    // either x is greater curr->value or curr->left == NULL 

    node *newNode = new node; // allcat new node
    newNode->left = newNode->right = NULL; // <- this schould be done by a constructor of node
    newNode->value = x;

    if ( x < curr->value )
    {
        // curr->left == NULL => new node is new start of list
        curr->left = newNode;
        newNode->right = curr;
    }
    else if ( curr->right == NULL )
    {
        // new node is new end of list
        curr->right = newNode;
        newNode->left = curr;
    }
    else
    {
        // new node someweher in the list
        node *rightNode = curr->right;
        curr->right = newNode;
        newNode->right = rightNode;
        newNode->left = curr;
        rightNode->left = newNode;
    }
}

Note, all the nodes you allocated with new you have to delete if you destroy them.

If you like to print your list from begin to the end you don't need a recursive function and avoid to copy your nodes. Use pointers:

void out(const node *ro)
{
    if ( ro == NULL )
        return;

    while ( ro->left != NULL )
        ro = ro->left;

    while ( ro != NULL )
    {
        cout << ro->value;
        ro = ro->right;
    }
}

...

out(&root);
answered on Stack Overflow Jan 2, 2016 by Rabbid76 • edited Sep 1, 2018 by Rabbid76

User contributions licensed under CC BY-SA 3.0