1

I'm coding a TreeStructure (TS) class, which lets me create parent and child objects, which are linked to each other. Every TS -object has m_parent attribute, which is controlled with parent property, and also they have children list, which holds in all the children of this parent. Whenever I add a child to it's parent's children list, it gets added to it's own children list too? Here's what I have:

ROOT = "__ROOT__"

class TreeStructure:

    def __init__(self, parent=ROOT, children=[]):
        self.children = children
        self.parent = parent

    @property
    def parent(self):
        '''Returns m_parent'''
        if hasattr(self, "m_parent"):
            return self.m_parent
        else:
            return None

    @parent.setter
    def parent(self, parent=ROOT):
        '''Sets m_parent'''
        if type(parent) == type(self):
            if self.parent:
                del self.parent
            self.m_parent = parent
            self.m_parent.children.append(self)
        elif parent == ROOT:
            if self.parent:
                del self.parent
            self.m_parent = ROOT
        else:
            raise TypeError("Parent's type %s did not match objects type %s"
                            %(type(parent), type(self)))

    @parent.deleter
    def parent(self):
        '''Deletes m_parent'''
        if self.parent:
            if self.parent != ROOT:
                self.m_parent.children.remove(self)
            del self.m_parent

And now by creating two simple objects, it should work. However, it doesn't.

a = TreeStructure()
b = TreeStructure(a)

The problem appears at line 25, self.m_parent.children.append(self). If I add print's to both sides of that line, I see that both print(self.m_parent.children) and print(self.children) print an empty list [] BEFORE the append line. Now if I add the prints AFTER the append line, both prints will say [<__main__.TreeStructure object at 0x...>], which should only happen for the parent, not the child?

Eric
  • 95,302
  • 53
  • 242
  • 374
  • Having a default argument on a "setter" doesn't make any sense, as it's always called with both arguments. Also, checks like ``type(parent) == type(self)`` look pretty scary and make subclassing impossible. Wouldn't ``isinstance(parent, TreeStructure)`` suffice? Or even ``hasattr(parent, "children")``. – lqc Nov 17 '12 at 14:04
  • I don't quite get your point here, wouldn't `isinstance(parent, TreeStructure)` just prevent me from inheriting any classes off of TreeStructure, cause that check would only be true with the superclass? **Edit**: Hmm just tested it with a subclass, my object is instance of the subclass, but still isinstance(object, superclass) returns true... Kinda stupid imo. Also, I'm creating a Menu -class here, and I only want to add Menu object's to other Menu objects, not TreeStructures, even if the Menu inhertis from TS. And `hasattr` would be pretty dangerous. –  Nov 17 '12 at 14:07
  • In this case, I would probably make a separate method like ``can_contain`` for that check. In ``TreeStructure`` this would accept any subclass, then in ``Menu`` you can override it to accept only ``Menu`` subclasses. This way if you later create a ``FancyMenu`` class, you will be able to add it to the menu without having to change this code. – lqc Nov 17 '12 at 14:23
  • And that's a perfect idea. I'm still learning and can't really think of something like that :P Thanks for the quick-tip :) –  Nov 17 '12 at 16:02

2 Answers2

5

Do not use [] as a default value!

>>> def bad(default=[]):
...     default.append(1)
...     print default
... 
>>> bad()
[1]
>>> bad()
[1, 1]
>>> bad()
[1, 1, 1]
>>> def good(default=None):
...     if default is None:
...             default = []
...     default.append(1)
...     print default
... 
>>> good()
[1]
>>> good()
[1]

Default arguments are created when the function is defined, not when it is called. So only use non-mutable types for defaults. using integers, strings, tuples is okay, but if you want a default list or dictionary or anything mutable then use None and do the above trick.

Read this question and answers for better understanding of this matter.

Community
  • 1
  • 1
Bakuriu
  • 98,325
  • 22
  • 197
  • 231
  • Woooah I've been using python for quite a lot, but didn't know this one, I've never really ran into the problem. But yeah thanks, I'm an inch more intelligent now :) –  Nov 17 '12 at 13:57
  • @Mahi: the way to detect this problem is by noticing `a.children is b.children == True` – Eric Nov 17 '12 at 13:58
  • Yeah I did detect the problem, just couldn't figure out why it happens, feeling stupid now :P –  Nov 17 '12 at 14:03
-2

You've been caught out by old-style classes (which don't support descriptors, such as @propertys). You should be using:

class TreeStructure(object):

After doing that, your code works for me

EDIT:

I'm testing using python 2.7. In 3.x, all classes are new-style.

Community
  • 1
  • 1
Eric
  • 95,302
  • 53
  • 242
  • 374
  • Yes, I'm using 3.2.3. That code shouldn't work for you either. –  Nov 17 '12 at 13:58
  • @Mahi: Yep, I was seeing an altogether different problem, where `children` was always `[]`. Added python 3.x as a tag, since it's important here. – Eric Nov 17 '12 at 13:59