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

[Adelein and Jeremy]

I have recently completed a mini-project with the goal of rewriting
in Python Eric Raymond's fortune.pl script
(http://www.catb.org/~esr/fortunes/fo...onfuse-apache),
except that instead of generating a sigline for mail, it prints a
fortune to screen in the traditional manner of the Unix fortune. I
have succeeded in terms of functionality, but because I am only a
novice programmer, I would appreciate any comments, criticism,
suggestions, alternative options, etc. that more experienced
programmers would be willing to give, whether technical or stylistic
in nature. Here is my code:

#! /usr/bin/env python

## fortune
## Jeremy Conn
## Version 0.9, 20020329
## GNU GPL http://www.fsf.org/licenses/gpl.html
## Description: Generates a random fortune for display onscreen from
## a single file of quotes which the user maintains; the
## quotes can be multi-line, and are separated by lines
## containing only a percent sign (same format as
## traditional fortune files).

import random
import re
import sys

FORTUNES_FILE = ".fortune"

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

# Let's see if we can open that file for reading
try:
fi = open(fortunes_f ile, 'r')
except:
sys.exit("Canno t open fortunes file %s." % fortunes_file)

# Collect the file pointers to each fortune entry in the file
fp_entry = [0]
line = fi.readline()
while line != "":
if re.match(r'^%$' , line):
fp_entry.append (fi.tell())
line = fi.readline()

# Seek to a random entry
try:
fi.seek(random. choice(fp_entry ))
except:
sys.exit("Canno t seek.")

# Add the entry to output message and then print it
fortune = ''
line = fi.readline()
while line != '':
if re.match(r'^%$' , line):
break
fortune += line
line = fi.readline()
print fortune

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

- Jeremy
adeleinandjerem y@yahoo.com

_______________ _______________ ____
Do you Yahoo!?
Yahoo! Finance Tax Center - File online. File on time.

http://taxes.yahoo.com/filing.html

[P@draigBrady.com]

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

Adelein and Jeremy wrote:[color=blue]
> I have recently completed a mini-project with the goal of rewriting
> in Python Eric Raymond's fortune.pl script
> (http://www.catb.org/~esr/fortunes/fo...onfuse-apache),[/color]

I did a very quick fortune file parser as part of:

http://www.pixelbeat.org/rotagator

Pádraig.

[Joakim Hove]

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


[color=blue]
> I would appreciate any comments, criticism, suggestions, alternative
> options, etc.[/color]

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

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

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_fortun es_file' instead of FORTUNES_FILE.

[color=blue]
> fp_entry = [0][/color]

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

fp_entry = []


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

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>
[color=blue]
> # Return fp to beginning of file, close the file, and exit program
> fi.seek(0,0)
> fi.close()
> sys.exit()[/color]

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@bccs.no / (55 5) 84076 | \
| Unifob AS, Avdeling for Beregningsviten skap (BCCS) | Stabburveien 18 |
| CMU | 5231 Paradis |
\ Thormøhlensgt.5 5, 5020 Bergen. | 55 91 28 18 /
\--------------------------------------------------------------------/

[Peter Otten]

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

Adelein and Jeremy wrote:
[color=blue]
> I have recently completed a mini-project with the goal of rewriting
> in Python Eric Raymond's fortune.pl script
> (http://www.catb.org/~esr/fortunes/fo...onfuse-apache),
> except that instead of generating a sigline for mail, it prints a
> fortune to screen in the traditional manner of the Unix fortune. I
> have succeeded in terms of functionality, but because I am only a
> novice programmer, I would appreciate any comments, criticism,
> suggestions, alternative options, etc. that more experienced
> programmers would be willing to give, whether technical or stylistic
> in nature. Here is my code:
>
> #! /usr/bin/env python
>
> ## fortune
> ## Jeremy Conn
> ## Version 0.9, 20020329
> ## GNU GPL http://www.fsf.org/licenses/gpl.html
> ## Description: Generates a random fortune for display onscreen from
> ## a single file of quotes which the user maintains; the
> ## quotes can be multi-line, and are separated by lines
> ## containing only a percent sign (same format as
> ## traditional fortune files).
>
> import random
> import re
> import sys
>
> FORTUNES_FILE = ".fortune"
>
> # What file should we use?
> if len(sys.argv) > 1:
> fortunes_file = sys.argv[1]
> else:
> fortunes_file = FORTUNES_FILE
>
> # Let's see if we can open that file for reading
> try:
> fi = open(fortunes_f ile, 'r')
> except:[/color]

Make it a rule to catch something, e. g.

try:
...
except IOError:
...

That reduces the chance that an unexpected error is mapped to something
else.

[color=blue]
> sys.exit("Canno t open fortunes file %s." % fortunes_file)
>
> # Collect the file pointers to each fortune entry in the file
> fp_entry = [0]
> line = fi.readline()
> while line != "":
> if re.match(r'^%$' , line):
> fp_entry.append (fi.tell())
> line = fi.readline()
>
> # Seek to a random entry
> try:
> fi.seek(random. choice(fp_entry ))
> except:
> sys.exit("Canno t seek.")
>
> # Add the entry to output message and then print it
> fortune = ''
> line = fi.readline()
> while line != '':
> if re.match(r'^%$' , line):
> break
> fortune += line
> line = fi.readline()
> print fortune
>
> # Return fp to beginning of file, close the file, and exit program
> fi.seek(0,0)[/color]

Code has no effect. I would remove it.
[color=blue]
> fi.close()
> sys.exit()[/color]

No need to call sys.exit() here.

My impression is that you are messing with file "pointers" too much which I
would consider rather low-level. For C this is OK as it has no decent
support for strings, but in Python I would suggest to relax and process the
whole file as a string or list of lines.

Here's a different approach that reads fortunes one at a time with a
generator function that collects lines until it encounters a "%\n".
I'm using an algorithm to choose a random fortune that was posted on c.l.py
by Paul Rubin, see

http://mail.python.org/pipermail/python-list/2003-December/201114.html

import random, sys

def fortunes(infile ):
""" Yield fortunes as lists of lines """
result = []
for line in infile:
if line == "%\n":
yield result
result = []
else:
result.append(l ine)
if result:
yield result

def findfortune(fil ename):
""" Pick a random fortune from a file """
for index, fortune in enumerate(fortu nes(file(filena me))):
if random.random() < (1.0 / (index+1)):
chosen = fortune

return "".join(cho sen)

if __name__ == "__main__":
print findfortune(sys .argv[1]),

As to error handling, I admit I'm lazy, but for small scripts I'm usually
happy with the traceback.

Peter

[Mel Wilson]

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

Or I could see (untested code)


import random, sys

def fortunes (infile):
return infile.read().s plit ('\n%\n')

def findfortune (filename):
return random.choice (fortunes (file (filename, 'rt'))

if __name__ == '__main__':
print findfortune (sys.argv[1])


Regards. Mel.

[Peter Otten]

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

Mel Wilson wrote:
[color=blue]
> Or I could see (untested code)
>
>
> import random, sys
>
> def fortunes (infile):
> return infile.read().s plit ('\n%\n')
>
> def findfortune (filename):
> return random.choice (fortunes (file (filename, 'rt'))
>
> if __name__ == '__main__':
> print findfortune (sys.argv[1])[/color]

That's what I meant with "in Python I would suggest to relax and process the
whole file as a string or list of lines" in my above post. The point of my
solution is to never have (a) the whole file in memory and (b) to make a
random choice from a set of (initially) unknown size while you go through
it.

Peter