list/classmethod problems

[Scott David Daniels]

ahart wrote:
> I'm pretty new to python and am trying to write a fairly small
> application to learn more about the language. I'm noticing some
> unexpected behavior in using lists in some classes to hold child
> objects. Here is some abbreviated code to help me explain.
> 
> When I run this script, I expect to see the following string printed:
> 
>     "<parent><item>one</item><item>two</item></parent>"
> 
> Instead, I see the following:
> 
> 
> "<parent><item>one</item><item>two</item><item>three</item></parent>"
> 
> Apparently, the p1 instance somehow thinks that the i3 instance is in
> its list. The i3 instance should instead be in the list for p2. By the
> way, when I call the __str__() method of p2, I get the same results as
> when I do it for p1. The list appears to be acting as if it were a
> static member - which it is not.
> 
> I do have some @classmethod methods in these classes in my complete
> script. Would that confuse the interpreter into thinking that other
> members are also static?
> 
> I can't seem to find any information on this problem. Does anyone have
> any ideas?

First, a few kvetches:

(1) Submit the _actual_code_ when you have problems, not paraphrases:
 > class Item(object)
should have been (and probably was):
   class Item(object):

 >     def __str__(self):
 >         ...
 >         s += "</parent>"
should have been (and probably was):
       def __str__(self):
           ...
           s += "</parent>"
           return s

(2) You must have been a Java or C++ programmer.  Don't be so stingy
     with access to your variables and methods (all the __xxx methods
     and variables).  Just use a single underscore (at least for the
     methods) telling users, "don't muck with this."  The double
     underscores make figuring out what's going interactively awkward.

(3) The if statement takes a boolean expression, not parens around a
     boolean expression.  "if a > b:" is the way to write a test, not
     "if(a>b):" (which is why I think Java or C++).

And now to your actual question:

 > ####################################
 > class Item(object)
 >     __text = ""

The line above is unnecessary (and deceptive).  It sets a _class_
variable named _Item__text to a zero-length string.

 >     def __get_text(self):
 >         return self.__text
 >     def __set_text(self, value):
 >         self.__text = value
 >     text = property(fget=__get_text, fset=__set_text)
 >
 >     def __init__(self, text=""):
 >         self.__text = text
This line sets an _instance_ variable named _Item__text to the arg.
 >
 >     #...some other methods here...
 >
 > class Parent(object):
 >     __items = []
The line above is unnecessary (and deceptive).  You want
a list-per instance, but you are accessing the class variable below

 >     def __get_items(self):
 >         return self.__items
 >     items = property(fget=__get_items)
 >
 >     def addItem(self, item):
 >         """Adds an Item object to the internal list."""
 > 	self.__items.append(item)
 >
 >     def __init__(self, items=[]):
 >         if(len(items)>0):
 >             for item in items:
 >                 self.addItem(item)

If you wrote this method as either:
       def __init__(self, items=[]):
           self.__items = []
           for item in items:
               self.addItem(item)
or:
       def __init__(self, items=[]):
           self.__items = list(items)

You would have the effect you want -- a list per instance.
By the way,
 >         if(len(items)>0):
 >             for item in items:
 >                 self.addItem(item)
Two complaints about this:
First, the "Pythonic way to test for a non-zero length list is "if var"
Second, even if the test were:
 >         if items:
 >             for item in items:
 >                 self.addItem(item)
That code is just plain slower (and less clear) than:
           for item in items:
               self.addItem(item)
If items is zero length, it goes through the loop body zero times.
 >
 >     def __str__(self):
 >         s = "<parent>"
 >         for item in self.__items:
 >             s += "<item>%s</item>" % item.text
 >         s += "</parent>"
 >
 >     #...some other methods here...
 >
 > if(__name__=="__main__"):
 >     i1 = Item("one")
 >     i2 = Item("two")
 >     i3 = Item("three")
 >
 >     p1 = Parent([i1, i2])
 >     p2 = Parent([i3])
 >
 >     print str(p1)
 > ####################################
 >
Now you get the output you expect.

By-the-by, why shouldn't the first class be simply?:

    class Item(object)
        def __init__(self, text=""):
            self.text = text

Finally, defining __repr__ rather than __str__ will make it
easier to fiddle with your code interactively w/o keeping anything
from working.

--Scott David Daniels
scott.daniels at acm.org