HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Possible refactoring in binary tree

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
refactoringpossiblebinarytree

Problem

I implemented a binary tree in Python, with preorder/postorder/inorder traversal methods.

```
class BinaryTree(object):

def __init__(self, rootObj):
self.key = rootObj
self.leftChild = None
self.rightChild = None

def insertLeftChild(self, newNode):
if self.leftChild == None:
self.leftChild = BinaryTree(newNode)
else:
newBinaryTree = BinaryTree(newNode)
newBinaryTree.leftChild = self.leftChild
self.leftChild = newBinaryTree
return self.leftChild

def insertRightChild(self, newNode):
if self.rightChild == None:
self.rightChild = BinaryTree(newNode)
else:
newBinaryTree = BinaryTree(newNode)
newBinaryTree.rightChild = newNode
self.rightChild = newBinaryTree
return self.rightChild

def getRoot(self):
return self.key

def setRoot(self, obj):
self.key = obj

def getLeftChild(self):
return self.leftChild

def getRightChild(self):
return self.rightChild

def __str__(self):
return '%s' % self.key

# DFS traversal techniques.
@classmethod
def preorder(cls, node):
if node == None: return False
print node
cls.preorder(node.getLeftChild())
cls.preorder(node.getRightChild())

@classmethod
def inorder(cls, node):
if node == None: return False
cls.inorder(node.getLeftChild())
print node
cls.inorder(node.getRightChild())

@classmethod
def postorder(cls, node):
if node == None: return False
cls.inorder(node.getLeftChild())
cls.inorder(node.getRightChild())
print node

def buildTree():
root = BinaryTree('a')
firstLeft = root.insertLeftChild('b')
firstRight = root.insertRightChild('c')
firstLeft.insertRightChild('d')
firstRight.insertLeftChild('e')
firstRight.insertRightChild('f')
return root

if __

Solution

As evidenced by the bug in Rev 1, the inconsistent naming between getRoot()/setRoot() and self.key is a trap that you've created for yourself.

Getters and setters would be better done using the @property decorator. Follow the convention in the documentation:

class BinaryTree(object):
    def __init__(self, obj):
        self.obj = obj
        self.left_child = self.right_child = None

    @property
    def obj(self):
        """The datum stored in the node."""
        return self._obj

    @obj.setter
    def obj(self, value):
        self._obj = value

    def __str__(self):
        return str(self.obj)    # <-- Simpler than %s interpolation

    …


Your traversal algorithms should not have a print statement, as they would be useless for anything other than printing to sys.stdout. Instead, they should yield each result. The caller can then choose to print them or do whatever it pleases.

for obj in pre_order(root):
    print obj


A nice way to write simple unit tests and provide documentation at the same time is to use the doctest module.

Code Snippets

class BinaryTree(object):
    def __init__(self, obj):
        self.obj = obj
        self.left_child = self.right_child = None

    @property
    def obj(self):
        """The datum stored in the node."""
        return self._obj

    @obj.setter
    def obj(self, value):
        self._obj = value

    def __str__(self):
        return str(self.obj)    # <-- Simpler than %s interpolation

    …
for obj in pre_order(root):
    print obj

Context

StackExchange Code Review Q#68341, answer score: 5

Revisions (0)

No revisions yet.