[Tutor] Program review

Kent Johnson kent37 at tds.net
Sat Jan 5 04:14:56 CET 2008


Ricardo Aráoz wrote:

>         except Exception, e :
>             logging.error('No pude leer "config.cfg" : %s', e.strerror)

You might like to use
   logging.error('No pude...', exc_info=True)
which will include the full traceback in the log. You can use the 
exc_info keyword with any of the logging commands.

>         mails = enumerate(addr.strip() for addr in fIncl
>                     if addr.strip() not in (excl.strip() for excl in fExcl))

I don't think this is doing what you want. (excl.strip() for excl in 
fExcl) will execute for each addr. But fExcl is a file which can only be 
iterated once, so only the first address will actually be checked.

I would write this as

   addrExcl = set(excl.strip() for excl in fExcl)
   mails = enumerate(addr.strip() for addr in fIncl
                     if addr.strip() not in addrExcl)

or even iterate over fIncl directly though you would have to change the 
way you count:
   for addr in fExcl:
     if addr.strip() in addrExcl:
       continue
     ...


You might consider adding a method to Mensaje that creates the message, 
rather than having Correo.__init__() read all the data values from 
mensaje. The current implementation has a code smell or two:
http://c2.com/cgi/wiki?FeatureEnvy and Data Class

Attribute names conventionally begin with lower case. If you must use 
upper case, at least be consistent!

Kent


More information about the Tutor mailing list