approach to writing functions

[Joe Mason]

In article <40290671.7A5864CA at engcorp.com>, Peter Hansen wrote:
> Bart Nessux wrote:
>> 
>> fp0 = os.popen("/sbin/ifconfig en0 inet", "r")
>> fp1 = os.popen("/usr/bin/uptime", "r")
>> fp2 = os.popen("/usr/bin/uname -a", "r")
>> fp3 = os.popen("/usr/bin/wc -l /etc/passwd", "r")
>> msg = MIMEText("-- IFCONFIG --\n\n" + fp0.read() + "\n-- UPTIME --\n\n"
>> +  fp1.read() + "\n-- UNAME --\n\n" + fp2.read() + "\n-- PASSWD LC
>> --\n\n" + fp3.read())
>> fp0.close()
>> fp1.close()
>> fp2.close()
>> fp3.close()
> 
> This sequence has duplication, so it could stand some refactoring.  Write
> a routine that executes a command and returns the result, then call it
> with various commands, something like this:
> 
> def cmd(c):
>     f = os.popen(c, 'r')
>     try:
>         result = f.read()
>     finally:
>         f.close()
>     return result
> 
> responses = [cmd(c) for c in
>     ['/sbin/ifconfig en0 inet', '/usr/bin/uptime', '/usr/bin/uname -a',
>      '/usb/bin/wc -l /etc/passwd']]
> 
> msg = MIMEText('blah %s, blah %s, blah %s, blah %s' % responses)
> 
> That's only one example of what you could do to simplify/make reusable
> code.
> 
> The best thing I know of to decide when to make a function is when you have
> code that is about to take responsibility for two different things.  Your
> code has responsibility for setting up user info, retrieving command output,
> execute four commands, generating the mail body from the results, 
> setting up the entire mail message, and sending the mail message.  That's
> six different areas of responsibility (give or take) and there should
> probably be at least three or four functions in there to handle some
> of those in a cleaner fashion.

Hee hee.  As the guy who disagreed with Terry before on philosophy, I'm
going to agree with him now on practicals.  There's no reason to split
up this code into three or four functions.  The only bit I don't like is
having all the read() calls inside the MIMEText string where it's easy
to miss them.  I would use Peter's cmd function, and just replace "fp? =
os.open(...)" with "r? = cmd(...)", and then change all the variable
names in MSG.  (If I were writing it for the first time, I might use
that loop comprehension depending on how many commands there are.  It's
a very useful shortcut if you're doing the same thing a lot, but it's
one more layer of comprehension you have to look past to see what the
code is doing.)

Joe