I want to create a binary tree from the following list, where in each line, the first number indicates the value to be stored at the node, second number indicates if the node has a left child (1 if yes, 0 if no) and third number indicates if the node has a right child:
297 1 1
319 0 1
124 1 1
282 0 0
530 1 1
424 0 0
287 1 1
214 0 0
471 0 0
376 0 1
173 0 0
So the tree should look like this:
Traversals of the tree should look like this:
Preorder: 297 319 282 124 530 287 471 376 173 214 424
Inorder: 319 282 297 471 287 376 173 530 214 124 424
But my program gives this output:
Here is my code. I have created an array of structures to represent the list:
struct node_info{
int element;
int leftbit;
int rightbit;
};
struct node_info node_list[50];
Code describing tree node and new node:
struct Node {
int data;
int ref_idx;
struct Node *left, *right;
};
Node* newNode(int data, int idx){
Node* temp = new Node;
temp->data = data;
temp->ref_idx = idx;
temp->left = temp->right = NULL;
return temp;
}
Below is the code to insert a new node. I think this is where the problem is:
void insertNode(struct Node* root, int list_idx){
if(root->left == NULL && node_list[root->ref_idx].leftbit == 1){
root->left = newNode(node_list[list_idx].element, list_idx);
return;
}
else if(root->right == NULL && node_list[root->ref_idx].rightbit == 1){
root->right = newNode(node_list[list_idx].element, list_idx);
return;
}
else if(root->left != NULL){
insertNode(root->left, list_idx);
}
else if(root->right != NULL){
insertNode(root->right, list_idx);
}
else{
cout << "\nInvalid case detected!\n";
}
}
This is the main function where I give the inputs and build the tree. Notice that I have added the root node manually while using the insertNode function to add all the other nodes:
int main(){
node_list[0].element = 297;
node_list[0].leftbit = 1;
node_list[0].rightbit = 1;
node_list[1].element = 319;
node_list[1].leftbit = 0;
node_list[1].rightbit = 1;
node_list[2].element = 124;
node_list[2].leftbit = 1;
node_list[2].rightbit = 1;
node_list[3].element = 282;
node_list[3].leftbit = 0;
node_list[3].rightbit = 0;
node_list[4].element = 530;
node_list[4].leftbit = 1;
node_list[4].rightbit = 1;
node_list[5].element = 424;
node_list[5].leftbit = 0;
node_list[5].rightbit = 0;
node_list[6].element = 287;
node_list[6].leftbit = 1;
node_list[6].rightbit = 1;
node_list[7].element = 214;
node_list[7].leftbit = 0;
node_list[7].rightbit = 0;
node_list[8].element = 471;
node_list[8].leftbit = 0;
node_list[8].rightbit = 0;
node_list[9].element = 376;
node_list[9].leftbit = 0;
node_list[9].rightbit = 1;
node_list[10].element = 173;
node_list[10].leftbit = 0;
node_list[10].rightbit = 0;
struct Node* test_root = newNode(node_list[0].element, 0);
for(int i = 1; i < 11; i ){
insertNode(test_root, i);
}
cout << "\nPreorder: ";
printPreorder(test_root);
cout << "\nInorder: ";
printInorder(test_root);
}
What exactly is wrong with my insertNode function? Note that I want my program to be able to handle any kind of input so I cannot build the tree manually and therefore need a function to find where to place each node.
CodePudding user response：
I think you have made this way to hard for yourself.
Fixing the code you provided is going to be hard. You would need to write this in a way that retained information between calls to insertNode() to remember where you had reached previously to understand where you need to insert the next value. Simply returning the "last" node is not enough, as you need to be able to go back up the tree at some point.
You have defined your input in a bread first order. So the standard technique for solving that is to push each where new values go onto a stack and use that to keep track of your position.
I am sure it can be done (but its hard).
Also in my opinion ugly.
So I would suggest you use an alternative solution (see below).
Let us give it a go at solving your problem:
// This is the state you need to remember
// between calls to insertNode()
std::list<Node**> nextNode;
// This is the root of your tree.
struct Node* root = nullptr
// Setup. Push the root as the first node.
nextNode.push_front(&root); // The location of the first node.
// Now lets loop over your input.
for(int i = 0; i < 11; i ){
if (nextNode.empty()) {
// ERROR
}
// Pull out the pointer we want to update.
Node** node = nextNode.pop_back();
// Create the node for that pointer.
(*node) = insertNode(ode_list[i].element);
// Push the pointers we want to assign.
if (node_list[i].leftbit) {
nextNode.push_front(&((*node)->left));
}
if (node_list[i].rightbit) {
nextNode.push_front(&((*node)->right));
}
}
if (!nextNode.empty()) {
// ERROR
}
I would redesign the input format into something that can be easily parsed recursively. At the heart of this is a Node. So you should define a format that represents a node.
Node => Integer Left Right
The main issue is that Left and Right are recursively pointing toanother Node or potentially null. So you have to be able to distinguish this. So When you start reading Left you have to know if it is null or a value. So let us introduce X as the null value in out input as this does not conflict with the alternative which is Integer.
So now we define the input as:
Node => 'X' // The null node
=> Integer Node(left) Node(right) // A value followed by two nodes.
So we can now represent your tree like this:
297 319 X 282 X X 124 530 287 471 X X 376 X 173 X X 214 X X X X
Let's try and break that up visually:
297 319 X 282 X X 124 530 287 471 X X 376 X 173 X X 214 X X 424 X X
V -----L------- -----------------R-------------------------------
// The left sub-tree from Root
// This is easier to visualize
// A value of 319
// left is null
// right is another sub-tree with 282 and two nulls.
319 X 282 X X
V L ---R---
// The right sub-tree from Root
// Not as easy to see (but I leave you home work
// to see if you can break that down by hand.
124 530 287 471 X X 376 X 173 X X 214 X X 424 X X
V --------------L---------------------- ---R---
With this format it becomes relatively easy to read as the function; read a node simply reads an X and returns null, or reads a value creates a node and then recursively tries to read the left and right values.
Node* readNodeFromStream(std::istream& stream)
{
char next;
if(stream >> next) {
if (next == 'X') {
return nullptr;
}
}
else {
// There is an error reading from the stream
// Take appropriate action.
}
stream.unget(); // Put last character that was not an X
// back on the stream so we can read it
// as part of the number.
int value;
if (stream >> value) {
Node* result = new Node{value};
result->left = readNodeFromStream(stream);
result->right = readNodeFromStream(stream);
return result;
}
else {
// Error reading from stream.
// Take appropriate action
}
}
int main()
{
std::string input = "297 319 X 282 X X 124 530 287 471 X X 376 X 173 X X 214 X X X X";
std::stringstream instream(input);
Node* root = readNodeFromStream(instream);
}
CodePudding user response：
As I write this, you have one person who addresses reading your data. But that doesn't explain your error.
Consider the path to add 530. By the time you add 530, you're going to proceed down the left path to the 282, which can't take children, and that leaves you with nowhere to put it.
You need to think of your insert code as "try to insert" and return bool true if success, false if not. If it fails to be able to insert when you try the left path, you can then try the right path.
But right now, you go down to the 282, can't add children, and return that error because you make no effort to take another path on failure.
CodePudding user response：
Seems that @JosephLarson explain why your implementation doesn't work. If u want to parse format which you described in question (if not, so @Martin York offer other format and how to parse it) u cant do it recursively. You must do it level by level no subtree by subtree (recursive method). So i offer u to use queue to understand which will be parent for current. My implementation:
Node* insertNode(int list_idx){
static std::queue < Node* > q;
Node* new_node = newNode(node_list[list_idx].element, list_idx);
if (!q.empty()){
Node* parent = q.front();
q.pop();
if (parent->left == nullptr && node_list[parent->ref_idx].leftbit == 1){
parent->left = new_node;
} else if (parent->right == nullptr && node_list[parent->ref_idx].rightbit == 1){
parent->right = new_node;
} else {
std::cerr << "Something goes wrong";
exit(1);
}
} else {
// means that it's root and we have no parent
// we will return root several lines below
}
if (node_list[list_idx].leftbit == 1){
q.push(new_node);
}
if (node_list[list_idx].rightbit == 1){
q.push(new_node);
}
return new_node;
}
int main(){
// --- some initialization ---
// then action:
struct Node* test_root = insertNode(0);
for(int i = 1; i < 11; i ){
insertNode(i);
}
cout << "\nPreorder: ";
printPreorder(test_root);
cout << "\nInorder: ";
printInorder(test_root);
}