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)