[Tutor] Function design

[Dave Angel]

Luke Paireepinart wrote:
> On Mon, Oct 26, 2009 at 2:08 PM, Eduardo Vieira <eduardo.susan at gmail.com>wrote:
>
>   
>> Hello, I need your help in designing a function. I have created this
>> script to check out if certain column in some csv files has value "0":
>>
>> import csv
>>
>> def zerofound(csvfile, outputfile, lastcolumn ):
>>    """Finds columns with zero prices. Column have 0 index"""
>>    final = csv.writer(open(outputfile, 'wb'), dialect='excel')
>>
>>    reader = csv.reader(open(csvfile, 'rb'), dialect='excel')
>>    for row in reader:
>>        if '0' in row[:lastcolumn]:
>>            final.writerow(row)
>>
>>
>> if __name__ == '__main__':
>>    zerofound('pricesfrombv.csv', 'noprices_in_BV.csv', 4)
>>    zerofound('excelbv.csv', 'noprices_in_ExcelBV.csv', 6)
>>
>> My question is. Is it OK to create functions with no "returns"?
>> Basically what I did resembles a subroutine, right? How could I
>> redesign this to use "return"?
>>
>>     
> Yes, the general "rule of thumb" in Computer Science is that you try to have
> functions that either have side-effects or return values, but not both.  In
> practice people sometimes have both (eg. try to do some side-effects, if
> they work return a 0 and if they don't return an error code) but try to
> avoid it if possible.
>
> For example, string.strip() has a return value but no side-effects, because
> it does not modify the original string, it returns a new copy of the string
> that is stripped.  Whereas list.sort() has no return value (or it returns
> None, depending on how you look at it) because it modifies the _ORIGINAL
> LIST_ which is a side-effect.
>
> As for your function, try to define the side-effects and change them into a
> return value.
> An example of a side-effect is printing to the screen or writing to a file,
> not just modifying global scoped or input variables.
>
> *************SPOILER*****************
> Your side-effect is that you're writing rows to "final".  So you should get
> rid of all your "final" code and just return the list of rows.  I would also
> suggest renaming the function to find_zeros() and obviously you would pass
> just the input filename.  Then you would have another function
> write_rows(outfile, rows) and it would output the rows to outfile.  I feel
> that would be a much cleaner design.  write_rows probably won't have a
> return value since you're writing to a function (side-effect!)
> *****************************************
>
> Hope that helps!
> -Luke
>
>
>   
>> Thanks for your input,
>>
>> Eduardo
>> _______________________________________________
>> Tutor maillist  -  Tutor at python.org
>> To unsubscribe or change subscription options:
>> http://mail.python.org/mailman/listinfo/tutor
>>
>>     
>
>   
I agree with Luke's comments.  But I'd like to point out an apparent bug 
(I haven't tried the code, this is just by inspection).

You use the test
      if '0' in row[.....]

that's not going to check for a zero value, it's going to check for a 
zero digit character somewhere in the value.  So 504 would also be 
recognized, as well as 540.

Normally, if it's a numeric int field, you want to convert it to int, 
and check its value, rather than do string manipulations directly, since 
a value 05 might really be intended to be the same as 5.

DaveA