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

[Peter Otten]

Adelein and Jeremy wrote:

> 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/fortune.pl-unconfuse-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_file, 'r')
> except:

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.


>     sys.exit("Cannot 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("Cannot 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)

Code has no effect. I would remove it.

> fi.close()
> sys.exit()

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(line)
    if result:
        yield result

def findfortune(filename):
    """ Pick a random fortune from a file """
    for index, fortune in enumerate(fortunes(file(filename))):
        if random.random() < (1.0 / (index+1)):
            chosen = fortune

    return "".join(chosen)

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