3
\$\begingroup\$

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
\$\endgroup\$
  • \$\begingroup\$ Obligatory suggestion to follow the style guide (also, please review the indentation). \$\endgroup\$ – jonrsharpe Aug 2 '14 at 18:45
2
\$\begingroup\$

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.

|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.