mutable member, bug or ...

Bruno Desthuilliers bdesth.quelquechose at free.quelquepart.fr
Sun Jun 4 22:15:23 EDT 2006


Sambo a écrit :
> By accident I assigned int to a class member 'count' which was 
> initialized to (empty) string and had no error till I tried to use it as 
> string, obviously. Why was there no error on assignment( near the end ).

Python is dynamically typed - which means that it's not the name that 
holds type info, but the object itself. names are just, well, names...

BTW, I failed to see where you assigned an int to the class attribute 
'count'. I just saw a try to call a string - which should raise a TypeError.

> class Cgroup_info:

Do yourself (and anyone having to work on or with your code) a favour: 
use new-style classes (ie : inherit from 'object'). And FWIW, the 
convention for class names is CamelCase - preferably without MS-like 
hungarian annotation.

>    group_name = ""
>    count = "0"  #last time checked and processed/retrieved
>    first = "0"
>    last = ""
>    retrieval_type = ""        # allways , ask( if more than some limit), 
> none
>    date_checked = ""
>    time_checked = ""
>    new_count = ""
>    new_first = ""
>    new_last = ""
> # local storage maintanance vars
>    pointer_file = ""
>    message_file = ""
> #maintanance vars       cur_mess_num = 0
>    cur_mess_id = ""

All these are *class* attributes (meaning they belong to the class 
object itself, not to instances). I really doubt this is what you want. 
If you hope to have all the above as instance attributes, you must 
assign them to the instance (usually in the __init__() method, which 
main purpose is to initialize the newly created instance.

>    def __init__( self ):
>        group_name = ""

this creates a local variable 'group_name', bound to an empty string. 
Using the reference to the current instance (usually named 'self', and 
always passed in as first param) is *not* optional.

>        count = "0"  #last time checked and processed/retrieved

and this creates a local variable 'count', bound to string '0'.

Of course, both are lost as soon as the __init__() returns.

>    def get_count( self ):
>        print self.count, type( self.count )
>        return string.atoi( self.count, 10 )

the string module is mostly deprecated. Use str object methods instead - 
  or if you just want to create an int from it's representation as a 
string, int(self.count).

> class server_info:
>    def "(server_info::)"get_group_stat( self, grp ):

invalid syntax.

>               gr_info = Cgroup_info()

and a problem with indentation here.

>        gr_info.group_name = grp

Tthis create a new instance attribute "group_name" on the gr_info 
object. This instance attribute will shadow the class attribute of the 
same name.

Also, FWIW, if you always know the value for group_name when 
instanciating a Cgroup_info object, you might as well pass it to the 
initializer.

>        try:
>            ind = self.group_list.index( grp )

The common convention for indices in each and every language is 'i'. If 
you really want a meaningful name, then group_index would be better.

Also, for this kind of lookups, dicts are really faster than lists.

>        except ValueError:
>            gr_info.count(0)

I didn't see any method "count()" in the declaration of class 
CGroup_info. I saw an attribute named "count", but it was bound to a 
string - which is not callable.

>            return ( gr_info )

parenthesis here are useless (and FWIW, they would be just as useless in 
C++).

>        print ind
>        if len( self.group_list[ind].split() ) == 4:
>            gr_info.count = self.group_list[ind].split()[1]
>            gr_info.first = self.group_list[ind].split()[2]        
>            gr_info.last = self.group_list[ind].split()[3]

group_list[ind] is the same as grp, isn't it ? if so, using grp directly 
might be much more efficient *and* much more readable.

Also, you're calling 4 times the same method. This is highly 
inefficient. Try this instead:
        parts = grp.split()
        if len(parts) == 4:
          gr_info.count, gr_info.first, gr_info.last = parts[1:]


> else:
>            gr_info.count = gr_info.first = gr_info.last = "0"

This style of "chained assignment" can be a real gotcha in Python. In 
this case, it is safe since "0" is immutable. But using a mutable object 
instead would lead to probably unexpected results. Try this:

a = b = []
a.append(1)
print b

You have to understand that Python "variables" (the correct name is 
"bindings") are just the association (in a given namespace) of a name 
and a *reference* (kind of a smart pointer) to an object.

>        return( gr_info )   

Here's a possible rewrite of your code. It's certainly not how it should 
be done, but (apart from going into wild guesses) it's difficult to come 
up with anything better without knowing the specs. Anyway, it should 
help you grasp a more pythonic way to do things:

class GroupInfo(object):

    def __init__(self, group_name, count="0", first="0", last=""):
        self.group_name = group_name
        self.count = count  #last time checked and processed/retrieved
        self.first = first
        self.last = last

        self.retrieval_type = "" # always , ask( if more than some 
limit), none
        self.date_checked = ""
        self.time_checked = ""
        self.new_count = ""
        self.new_first = ""
        self.new_last = ""

        # local storage maintanance vars
        self.pointer_file = ""
        self.message_file = ""

        #maintanance vars
        self.cur_mess_num = 0
        self.cur_mess_id = ""

    # if you want to store count as string
    # but retrieve it as an int. The setter will
    # be used by the initializer, and will then create
    # the implementation attribute _count
    def _get_count(self):
        return int(self._count)

    def _set_count(self, value):
        self._count = str(value)

    count = property(_get_count, _set_count)


class ServerInfo(object):
     def __init__(self, ???)
         self._groups = {} # dict lookup is faster
         # ...
     def has_group(self, group_name):
         return self._groups.has_key(group_name)

     def get_group_stat(self, group_name):
         if not self.has_group(group_name):
             return GroupInfo(group_name, "0")

         parts = group_name.split()
         if len(parts) != 4:
             parts = [None, "0", "0", "0"]
         return GroupInfo(group_name, *(parts[1:4]))


As a last point, I'd second Fredrik : don't try to write C++ in Python. 
Learn to write Python instead. The (freely available) "Dive into Python" 
book should be a good place to get started.

HTH



More information about the Python-list mailing list