[Tutor] Program review

[Ricardo Aráoz]

Jeff Younker wrote:
>> Maybe it is my poor understanding of exception handling. My intention
>> here is to open the first file, if error then report in logging and
>> finish normally, else open the 2nd file, if error then report in  
>> logging
>> close the 1st file and finish normally. If no error then process.
>>
> 
> Right, but it helps to break up the error handling and the cleanup.
> Right now the error reporting exception handlers are intermixed
> with your cleanup error handlers.  Separating them makes the code
> clearer.
> 

Yes, but in such a simple program I'd rather have shallow level of
function/method callings. After all the pertinent code fits all in a
screen and is not hard to understand. If it was bigger or more
complicated I agree with you and I would re-factor it.

> def open_file(filename):
>       try :
>           return open(filename)
>       except Exception:
>           logging.error('No pude abrir "%s"' % filename, exec_info=True)
>           raise
> 
> # Goes into configuration class
> def mails(self):
>    fIncl = open_file(self.direcciones)
>    try:
>       fExcl = open_file(self.excluidas)
>       try:
>           return enumerate(
>                       set(addr.strip() for addr in fIncl)
>                       - set(excl.strip() for excl in fExcl))
>       finally:
>            fExcl.close()
>    finally:
>         fIncl.close()
> 

Both open_file assignments should go after the respective 'try:' statement.
Besides both 'finally:' blocks will be executed after an exception (i.e.
the file is not open) generating a second exception. Wrong use of
'finally:', it is executed even if an exception has occurred.
>From Python 2.5 Documentation :
"""
If finally is present, it specifies a `cleanup' handler. The try clause
is executed, including any except and else clauses. If an exception
occurs in any of the clauses and is not handled, the exception is
temporarily saved. The finally clause is executed. If there is a saved
exception, it is re-raised at the end of the finally clause. If the
finally clause raises another exception or executes a return or break
statement, the saved exception is lost.
"""

> Or if you're using python 2.5 then you can use with statements:
> 
> from __future__ import with_statements
> from contextlib import closing
> ...
> def mails(self):
>      with closing(open_file(self.direcciones)) as fIncl:
>             with closing(open_file(self.excluidas)) as fExcl:
>                 return enumerate(
>                         set(addr.strip() for addr in fIncl)
>                         - set(excl.strip() for excl in fExcl))
> 
>> close the 1st file and finish normally. If no error then process.
> 

How would that look with exception handling included? PEP 0343 is not an
example of clarity in the definition of a statement, it mixes
justification with historic development with definition with actual
equivalent code. Couldn't or wouldn't bother to understand it.

> If an exception is raised then the code terminates right there and
> then.   So, if procesar is written as:
> 
> def procesar(mensaje):
>       mails = mensaje.mails()
>       miCorreo = Correo(mensaje)
>       ....
> 
> then if mensaje.mails() raises an exception then the code
> terminates immediately, and the body of Correo will only be
> executed if there is no error.
> 
> As you have it written it terminates, but it also silently consumes the
> exception.  If something goes wrong with your program then there is
> no way of diagnosing the problem because the failure cause is never
> reported.

Yes, it is reported in the log. And that's exactly how I want it to be.
In my experience non tech users will go into panic if they see an error
screen with all that info up their faces. OTOH if they look at a log
file they seem to feel they have more control over the situation and
that what happened is not catastrophic.

Ricardo