On Fri, 30 Jun 2006, Madan S. wrote:
Hi Madan, thanks for the review! In your response, you snipped off
part of the diff:
def add_child(self, newchild):
+ child_already_exists = 0
if self.children is None: # if you're a file,
self.children =  # become an empty dir.
- child_already_exists = 0
- for a in self.children:
- if a.name == newchild.name:
- child_already_exists = 1
- if child_already_exists == 0:
- newchild.path = os.path.join (self.path, newchild.name)
+ for a in self.children:
+ if a.name == newchild.name:
+ child_already_exists = 1
> On Fri, 30 Jun 2006 04:55:50 +0530, <email@example.com> wrote:
> >- # If you already have the node,
> > else:
> >+ for a in self.children:
> >+ if a.name == newchild.name:
> >+ child_already_exists = 1
> >+ break
> >+ if child_already_exists:
> > if newchild.children is None:
> > # this is the 'end' of the chain, so copy any content here.
> > a.contents = newchild.contents
> >@@ -180,6 +177,9 @@
> > # try to add dangling children to your matching node
> > for i in newchild.children:
> > a.add_child(i)
> >+ else:
> >+ self.children.append(newchild)
> >+ newchild.path = os.path.join(self.path, newchild.name)
> This has changed the logic of the existing code. Now the function
> doesnt do add_child() of the newchild's children. Am wondering why you
> made this change.
I'm not seeing any place where the logic changed here, other than at
the beginning of the method, where we now avoid iterating over a newly
created, known-to-be-empty list of children for "newchild".
As I mentioned in the change log message, I removed the negative logic
from the conditional -- which I find makes for much more
comprehensible code -- in the second section of the method, which
necessitated that I swap the blocks for its "if" and the "else"
statements. The children of "newchild" are still added, but now in
the "else" block instead of the "if" block.
Received on Fri Jun 30 20:27:34 2006
- application/pgp-signature attachment: stored