[Tutor] Pythonese/Efficiency/Generalese critique please

[Javier Ruere]

Lee Cullens wrote:
> Pythonese/Efficiency/Generalese critique please
> 
> The utility produces a csv file of a specified directory tree for import 
> to a spreadsheet application.  My first attempt employed os.walk(), but 
> I found converting the results of such more troublesome than the 
> recursive approach of this version.

   I think it would simplify the logic of your program.

> Including the module code in this email puts it a little over the 40K 
> limit (lots of comments) so I uploaded it to my dotMac iDisk:
> http://homepage.mac.com/lee_cullens/dirtss.py.zip
> On a Mac (at least) the .py is recognized as an executable and warns of 
> such so Ctrl click (right click) and open with your programming text 
> editor.  It would fail to run anyway without arguments.  
> 
> I would appreciate any comments you see fit to offer.

   The first time I ran this script ("python dirtss.py Media/ out.csv"), 
it failed. It was confused by the slash after Media. I didn't even 
notice it since bash completed the name. In os.path there are functions 
for managing paths which avoid this and other problems.
   There is no error handling. The script fails when it meets a 
directory for which it has no access.
   Comments are very good but those refering to the behaviour of a 
function should be placed inside of it. I, personaly, do it like this:

# The following code is not tested.
def foo(a, b, c):
     """foo(a:int, b:str, c:sequence<str>):sequence

     Prepends b, a times to the strings in c.

     a: The number of times b will be repeated.
     b: A string to prepend a times to the strings is c.
     c: A sequence with strings.
     """
     return map(lambda x : (b * a) + x, c)

   String concatenations like this:

for i in range(alvl, clvl):
     csvline = csvline + '"' + pl[i] + '",'

should be replaced by something like the following:

cvsline += '"' + pl[alvl:clvl].join('",')

   Besides that everything else seems OK. I believe using os.walk and 
the csv module would simplify the code but I'm too lazy to rewrite the 
script myself.
   A note on style: I have never seen so many nested functions but I 
don't dislike it. :)
   A note on design: Maybe making the function a generator of tuples may 
simplify the code and separate processing from formatting and output. 
Maybe first making a better os.walk which flags entries as file, dir, 
empty dir, link, etc. would make a nice function you might use in the 
future.

Javier