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
funnydman
  • 9,083
  • 4
  • 40
  • 55
  • So, you really must provide a [mcve]. 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 – juanpa.arrivillaga Jul 13 '22 at 18:38
  • Is this a binary search tree? Where do you want to insert the key? – funnydman Jul 13 '22 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) – ARIANA MONDIRI Jul 13 '22 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.. – ARIANA MONDIRI Jul 13 '22 at 20:35

1 Answers1

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)
trincot
  • 317,000
  • 35
  • 244
  • 286