Try to implement comments tree for Django

Question

I've tried to create an implementation of a tree for comments. I want use it in Django to store comments. Please tell me how to implement it much simpler, without recursion, to find child nodes.

class commentTree(object):
def __init__(self, id, data, parentId = 0, childrens = None):
    "Represents one leaf Tree"
    self.id = id
    self.parentId = parentId
    self.title = data['title']
    self.body = data['body']
    self.childrens = []
    self.isLeaf = True if len(self.childrens) > 0 else False
    if childrens is not None:
        for children in childrens:
            self.childrens.append(children)

def addComment(self, parentId, data):
    root = 1 if self.isLeaf == False else 0
    comment = commentTree(parentId + root + len(self.childrens), data, self.id)
    if parentId == self.id:
        self.childrens.append(comment)
    else:
        node = self.findById(parentId)
        node.childrens.append(comment)

def getParent(self, node):
    "Gets parent node"
    if self.parent is not None:
        return self.parent

def getSize(self, node):
    "Returns size of childrens length + node"
    return len(node.childrens) + 1

def findById(self, id):
    def searchInChildren(id, childNodes):
        for children in childNodes:
            if children.id == id:
                return children
            else:
                searchInChildren(id, children)
    if id == self.id:
        return self
    else:
       result = searchInChildren(id, self.childrens)
       if result.id == id:
           return result
       else: return None

def getSiblings(self, node):
    "Get nodes that have one parent"
    nodes = []

    def getAllChildrens(self, child):
        for children in node.childrens:
            nodes.append(children)
            if children.isLeaf:
                getAllChildrens(self, children)

    getAllChildrens(self, node)
    return nodes

Answer 1

A few comments regarding style:

• As pointed out by @jonrsharpe

  • Please have a look at PEP8
  • Use for spaces for each indentation level

• Naming

  • Use capitalized words for class names (commentTree -> CommentTree)
  • Use underscores for methods (findById -> find_by_id)

• Do not use spaces around the equal sign to set default values for arguments (parentId = 0 -> parent_id=0)

• Review hardcoded booleans since usually they are not needed:

  self.isLeaf = True if len(self.childrens) > 0 else False
  self.is_leaf = len(self.children) > 0  # True if ... else False not needed
  
  root = 1 if self.isLeaf == False else 0
  root = 1 if not self.is_leaf else 0  # False not needed
  root = int(not self.is_leaf)  # boolean to int conversion
  

• Do not use builtin name functions for arguments (id)

• Note that children is already plural

Regarding the code:

• addComment method

  • What is the goal of the new commentTree object id calculation? Shouldn't be easier to get a new id from the database itself?
  • What is the benefit of being able to add a node as a child of a node that is not the node for which the method was called. This makes the API confusing in my opinion.

• getParent method

  • What is the expected returned value when self.parent is None?
  • If the expected value is None, then the whole method implementation could be return self.parent

• getSize method

  • Why is there a node parameter to this method? Shouldn't return the length of the tree object itself?
  • This method implementation should be recursive. Note that each child may have children on its own.

• findById method

  • Why don't you like the recursive implementation? Recursion is what makes them implementation easier and I don't think you need to handle more levels than allowed by the interpreter
  • In the recursion the base case, the one for a leaf node, is missing

  • I'd try something like this:

    def find_by_id(self, comment_id):
        """Find node by id."""
        if self.node_id == node_id:
            return self
    
        if self.is_leaf:
            return None
    
        for child in self.chidren:
            node = child.find_by_id(id)
            if node:
                return node
    

• getSiblings method

  • Why is there a node parameter to this method? Shouldn't return the siblings of the current node?
  • There is no need to implement any recursion since siblings are the nodes at the same level of the tree.

  • I'd try something like this:

    def get_siblings(self):
        """Get sibling nodes."""
    
        # Get parent (not implemented, but using find_by_id on the root node should work)
        parent = ...
    
        return [node for node in parent.children if node.id != self.id]
    

I hope this helps.