snippetpythondjangoMinor
Try to implement comments tree for Django
Viewed 0 times
djangoimplementfortrycommentstree
Problem
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 nodesSolution
A few comments regarding style:
-
As pointed out by @jonrsharpe
-
Naming
-
Do not use spaces around the equal sign to set default values for arguments (
-
Review hardcoded booleans since usually they are not needed:
-
Do not use builtin name functions for arguments (
-
Note that children is already plural
Regarding the code:
-
-
-
-
-
I'd try something like this:
-
-
I'd try something like this:
I hope this helps.
-
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
commentTreeobject 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.parentisNone?
- If the expected value is
None, then the whole method implementation could bereturn self.parent
-
getSize method- Why is there a
nodeparameter 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
nodeparameter 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.
Code Snippets
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 conversiondef 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 nodedef 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]Context
StackExchange Code Review Q#58839, answer score: 2
Revisions (0)
No revisions yet.