Python help

[Cole Tuininga]

First things first - bravo to you Erik for offering up some more general
(and IMHO) quite correct advice on approaching python scripts rather
than giving "quickie" advice like I did.

That said, there are two things in your post I wanted to respond to.

> A dict is a more straightforward way to use a 
> simple struct-like object, as Cole suggested.  The overhead of using 
> a class over a dict is not very significant, though, since Python's 
> classes are really just customized dict types behind the scenes.  

This may appear to be true for simple situations.  However, take a look
at http://www.bagley.org/~doug/shootout/bench/objinst/  This is
admittedly old data, using python 2.2, but should still give a general
idea.  Python is quite slow for object instantiation with large numbers
of objects.  Far less true for dictionaries.  

But it all depends on what your needs are, of course.

> But you don't need to know that.  And because dicts are used so 
> extensively in Python, they are ultra-opt!
> imized by the Python developers.  

Very true.  Paul - when you get a little more used to all things
pythonic, check out the setdefault and getdefault methods for
dictionaries.  Very slick.

[snip]


> #!/sw/bin/python
> 
> import os, sys
> 
> 
> class Group:


Next point - the following line has a bug:
    
>     def __init__(self, name="", gid="", members=[]):

This is a very subtle one.  The point is that in a method definition
within an object, you never want to assign a mutable type (namely, the
empty list) to an argument.  Weird behavior will occur.

Example:

#!/usr/bin/python

class bob:
	def __init__( self, list_ref = [] ):
		self.list_ref = list_ref

	def printListRef( self ):
		print str( self.list_ref )


b1 = bob()
b2 = bob()

b1.list_ref.append( "data" )

print "b1:",
b1.printListRef()
print "b2:",
b2.printListRef()

# End of script

When this is run, you will see:
./test.py
b1: ['data']
b2: ['data']

Hey, wait a minute?  We only assigned "data" into b1!  Why did this
happen?  The answer is again due to python's heavy use of references.

When you instantiate an object, you are copying a lot of items from the
class definition - including references to arguments.  

When I do b1 = bob(), I'm getting a reference to that empty list defined
in the __init__ method's signature.  When I do b2 = bob(), I'm getting a
reference to the exact *same* list.  So from then on, any instantiated
object will be sharing that list.

Instead, what you want is something like this:

	def __init__(self, name="", gid="", members=None):
		self.name = name
		self.gid = gid
		if members is None:
			self.members = []
		else:
			self.members = members

>         self.name = name
>         self.gid = gid
>         self.members = members
>         
>     def setValuesFromFileLine(self, line):

Can I recommend changing the following to
	record = line.strip().split(":")

>         record = line.split(":")
>         self.name = record[0]
>         self.gid = record[2]
>         self.members = record[3].split(",")
>         
>     def toLdifFormat(self):
>         self.members.sort()
>         output = ",".join(members)
>         return "%s:%s:%s" % (self.name, self.gid, output)
> 
> 
> 
> def main(args=sys.argv):

One point about the above line - if you're testing from a python prompt,
you'll need to pass your arguments in as a list.  So for instance,
>>> main( ["/etc/group"] )

rather than just

>>> main ( "/etc/group" )

Otherwise, the next line would generate some rather peculiar behavior.

>     f = file(args[1], "r")

Also, below, it looks like you started out wanting to use a list, but
switched to a dict:

>     groups = []
>     
>     for line in f:
>         g = Group()
>         g.setValuesFromFileLine(line)
>         groups_dict[g.name] = g
>     
>     f.close()
>     
>     for group in groups:
>         print group.toLdifFormat()

Instead, to pick a list which seems reasonable appropriate to me:

	groups = []

	for line in f:
		g = Group()
		g.setValuesFromFileLine( line )
		groups.append( g )

	f.close()

	for group in groups:
		print group.toLdifFormat()

> if __name__ == "__main__":
>     main()

These items aren't meant as bashing - I know you said you wrote it
without testing.  I'm just trying to clarify.  8)

-- 
"... one of the main causes of the fall of the Roman Empire was that,
 lacking zero, they had no way to indicate successful termination of
 their C programs."  --  Robert Firth

Cole Tuininga
Lead Developer
Code Energy, Inc
colet at code-energy.com
PGP Key ID: 0x43E5755D