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