0

I am trying to implement a Binary Tree using a queue. I am having an issue to set a new value for self.root who is initially set in class BinaryTree as None using the method InsertNode(self,data). I try to return self.root with it's new value but the value remains the same (None) Any Idea?

class BinaryTree:
    def __init__(self):
        self.root = None

    def InsertNode(self, data):
        newNode = Node(data)
        newNode = newNode.data

        print('self.root =', self.root)
        print('new node', newNode)

        if self.root == None:
            self.root = newNode

            print('self.root =', self.root)

        else:
            print('else won')
            queue = []
            # print(queue)
            queue.append(self.root)
            while True:
                node = queue.pop(0)
                if node.left != None and node.right != None:
                    queue.append(node.left)
                    queue.append(node.right)
                else:
                    if node.left == None:
                        node.left = newNode
                        queue.append(node.left)
                    else:
                        if node.right == None:
                            node.right = newNode
                            queue.append(node.right)
                break
        return self.root
4
  • So, you really must provide a minimal reproducible example. But in any case, glancing at what you have, newNode = newNode.data seems wrong. The node you created is now gone, and you simply have newNode = data, AFAIKT Commented Jul 13, 2022 at 18:38
  • Is this a binary search tree? Where do you want to insert the key? Commented Jul 13, 2022 at 18:45
  • @funnydman i am just trying to construct a simple binary tree. This is how i would insert elements BinaryTree().InsertNode(2) BinaryTree().InsertNode(4) BinaryTree().InsertNode(6) Commented Jul 13, 2022 at 20:27
  • @juanpa.arrivillaga I did this because otherwise when I try to print(newNode) it returns the location. Like this: new node <__main__.Node object at 0x00000241A306AFE0> and it doesn't seem to help me solve the problem I am facing.. Commented Jul 13, 2022 at 20:35

1 Answer 1

0

Some issues:

  • Right after creating a new node and assigning its reference to newNode, your code assigns the data of that node to newNode. At that point you lose the reference to your node, and it is forever lost. Moreover, while self.root is supposed to be node reference, you then continue to assign the data value to self.root. In short, don't do newNode = newNode.data

  • The while loop can only make one iteration, because it has an unconditional break. That break should be conditional: it should be indented one level more so it is placed in the else block.

With those two fixes, your code will work (provided the other code you have, like for class Node is error-free).

Not a problem, but you should also look at these points:

  • At the place where you have if node.right == None, this condition is always going to be true, given that it was already known that not both node.left and node.right are None, and also that node.left is None. So the only remaining possibility for node.right is that it is None.

  • Once you have assigned newNode to where it belongs, there is no need any more to append to the queue.

  • When using a list as a FIFO queue, it is better to use a deque and its popleft method.

  • In Python, names of methods are commonly written with a first lowercase letter. Either name your method insertNode or why not just insert?

  • As node.left is a node reference or None, you can just test for node.left instead of node.left != None. Similarly for node.right

  • Instead of testing for the end-condition inside the while True loop, you could change the order of your code a bit and make it a while node.left and node.right loop. Then the assignment of newNode to the right attribute can happen after the loop has finished.

  • The insert method does not need to return the root node. The reference of the root node is maintained in an attribute of the instance. The outside caller should in fact have no need to ever access node references. I would suggest not to return the root node.

Here is the resulting code. I added __repr__ implementations so the tree can be printed with some basic indentation format:

from collections import deque

class Node:
    def __init__(self, data):
        self.data = data
        self.left = self.right = None

    def __repr__(self, tab=""):
        return ((self.right.__repr__("  " + tab) if self.right else "")
              + tab + repr(self.data) + "\n"
              + (self.left.__repr__("  " + tab) if self.left else ""))
        
       
class BinaryTree:
    def __init__(self):
        self.root = None

    def insert(self, data):
        newNode = Node(data)

        node = self.root
        if not node:
            self.root = newNode
        else:
            queue = deque()
            while node.left and node.right:
                queue.append(node.left)
                queue.append(node.right)
                node = queue.popleft()
            if node.left:
                node.right = newNode
            else:
                node.left = newNode

    def __repr__(self):
        return repr(self.root).rstrip() if self.root else "<Empty>"

# Example run
tree = BinaryTree()
print(tree)
for i in range(1, 13):
    tree.insert(i)
print(tree)
Sign up to request clarification or add additional context in comments.

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.