0

I created Binary Tree (Abstract data type) using template .

#include <bits/stdc++.h>

using namespace std;


template <typename T>class BinaryTree{
public:
    T data;
    BinaryTree*first;
    BinaryTree*second;
    BinaryTree(){
        data=-1;
        first=nullptr;
        second=nullptr;
    }
    BinaryTree(T data){
        this->data=data;
        first=nullptr;
        second=nullptr;
    }
};
template <typename T>BinaryTree<T>*InputBinaryTree(){
   cout<<"Enter root element : "<<endl;
    T data;
    cin>>data;
    BinaryTree<T>*root=new BinaryTree<T>(data);
    queue<BinaryTree<T>*>childrenholder;
    childrenholder.push(root);
    while(!childrenholder.empty()){
                BinaryTree<T>*rooty=childrenholder.front();
                cout<<"Enter first child of "<< rooty->data <<" : "  <<endl;
                T first;
                cin>>first;
                cout<<"Enter second child of "<< rooty->data <<" : "  <<endl;
                T second;
                cin>>second;
                if(first!=-1){
                        BinaryTree<T> * firsty=new BinaryTree<T>(first);
                        rooty->first=firsty;
                        childrenholder.push(firsty);

                }
                if(second!=-1){
                        BinaryTree<T> * secondy=new BinaryTree<T>(second);
                        rooty->second=secondy;
                        childrenholder.push(secondy);
                }
                childrenholder.pop();
    }
    return root;
}
template <typename T>void OutputBinaryTree(BinaryTree<T>*root){
    cout<<root->data<<endl;
    queue<BinaryTree<T>*>childrenholder;
    childrenholder.push(root);
    while(!childrenholder.empty()){
            BinaryTree<T>*rooty=childrenholder.front();
            BinaryTree<T>*firsty=rooty->first;
            BinaryTree<T>*secondy=rooty->second;
            if(firsty->data!=-1){
                cout<<firsty->data<<" , ";
                childrenholder.push(firsty);

            }
            if(secondy->data!=-1){
                cout<<secondy->data<<" ";
                childrenholder.push(secondy);
            }
            cout<<endl;
            childrenholder.pop();
    }
}

int main(){
    BinaryTree<int>*root;
    root=InputBinaryTree<int>();
    OutputBinaryTree<int>(root);

}

class BinaryTree - Binary Tree class.

InputBinaryTree() - function used to get input for binary tree.

OutputBinaryTree() - function used to print binary tree.

childrenholder - queue is used to get element of children of a node.

I considered only Integer as Input. I got input for binary tree in level order wise and expect to print the binary tree in level order wise as well. I am getting error while executing OutputBinaryTree(). Especially on the looping statement of the OutputBinaryTree(). The looping statement doesn't seem pop the elements in childrenholder queue.

Input given:

Enter root element :
1
Enter first child of 1 :
2
Enter second child of 1 :
4
Enter first child of 2 :
-1
Enter second child of 2 :
3
Enter first child of 4 :
5
Enter second child of 4 :
6
Enter first child of 3 :
-1
Enter second child of 3 :
-1
Enter first child of 5 :
-1
Enter second child of 5 :
-1
Enter first child of 6 :
-1
Enter second child of 6 :
-1

-1 is assumed as no element is present at that node.

Output expected:

1
2 , 4
3 , 5 , 6

Output received:

1
2 , 4

exits.

  • 1
    Whomever who taught you about [that header file](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) should be taken out and shouted at! The `using namespace std;` statement is merely a [bad habit](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). – Some programmer dude Jul 14 '23 at 09:06
  • As for the rest of the code, I suggest that you split up the tree class into separate `Tree` and `Node` classes. Let `Node` be only a simple holder of the data, and links to the left and right side, nothing more. Keep operations in the `Tree` class. Also don't mix input into the `Tree` class, it should be done outside of it, and then have a simple `insert` function which inserts one data object. And don't use pointers for the `Tree` objects. – Some programmer dude Jul 14 '23 at 09:08
  • Your `data` type is `T`; you initialize it with `-1` in the default constructor, if `T` is a `std::string`, so you would have a problem with `data` initialization. – Jibel Jul 14 '23 at 09:08
  • @Someprogrammerdude Sorry for that. I am just learning and I'll try to change the way I code soon as possible. – Kharinandan D N Jul 14 '23 at 09:09
  • @Jibel for now please just consider only about the integer data type. – Kharinandan D N Jul 14 '23 at 09:10
  • Your output code assumes that "no child" is indicated by the child existing and having the value -1. Compare this assumption to what you're doing when inputting the tree. – molbdnilo Jul 14 '23 at 09:10
  • @molbdnilo first of all `rooty` which doesn't have children will print itself and it's children won't be pushed into the queue since it is not present. – Kharinandan D N Jul 14 '23 at 09:15
  • @KharinandanDN `data=-1;` -- No, it should be: `data = {};` this makes more sense, and will work with any type that has a default constructor. – PaulMcKenzie Jul 14 '23 at 09:54

1 Answers1

2

Your input section does not add a child if the user's input is -1; the child in that case is the null pointer, not a node with a -1 value.
Then your output section assumes that every valid node has two non-null children, "empty" indicated by a value of -1, and you dereference those pointers unconditionally.
And dereferencing a null pointer is a big no-no.

The output conditions should be

if (firsty != nullptr)

and

if (secondy != nullptr)

Also, try to create a BinaryTree<std::string> and see what happens.
(How many types T do you think can be converted from -1?)

molbdnilo
  • 64,751
  • 3
  • 43
  • 82