ESR's fortune.pl redone in python - request for critique

Joakim Hove hove at bccs.no
Tue Mar 30 06:08:22 EST 2004



> I would appreciate any comments, criticism, suggestions, alternative
> options, etc. 

Here are some random comments; needless to say they represent my
subjective view. Others mitht disagree heartily!


> FORTUNES_FILE = ".fortune"
>
> # What file should we use?
> if len(sys.argv) > 1:
>     fortunes_file = sys.argv[1]
> else:
>     fortunes_file = FORTUNES_FILE

Using the same variable name with only difference in capitalization I
consider very bad taste. I would suggest using for instance the
variable name 'default_fortunes_file' instead of FORTUNES_FILE.


> fp_entry = [0]

You probably want to start with an *empty* list, not a list with '0'
as the first element, i.e.

fp_entry = []



> fortune = ''
> line = fi.readline()
> while line != '':
>     if re.match(r'^%$', line):
>         break
>     fortune += line
>     line = fi.readline()
> print fortune

Wouldn't it be more natural to look for '%' instead of blank lines:

<Untested>
fortune = ''
line = fi.readline()
while line:
    if re.match(r'^%$',line):
        break
    fortune += line
    line = fi.readline()
</Untested>

> # Return fp to beginning of file, close the file, and exit program
> fi.seek(0,0)
> fi.close()
> sys.exit()

I like to close the files explicitly, like you do. However it is not
really necessary, they are closed automatically (by the garbage
collector I guess).

Repositioning the 




-- 
  /--------------------------------------------------------------------\
 / Joakim Hove  / hove at bccs.no  /  (55 5) 84076       |                 \
 | Unifob AS, Avdeling for Beregningsvitenskap (BCCS) | Stabburveien 18 |
 | CMU                                                | 5231 Paradis    |
 \ Thormøhlensgt.55, 5020 Bergen.                     | 55 91 28 18     /
  \--------------------------------------------------------------------/



More information about the Python-list mailing list