By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
455,534 Members | 1,299 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 455,534 IT Pros & Developers. It's quick & easy.

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

P: n/a
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_file, 'r')
except:
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)
fi.close()
sys.exit()

- Jeremy
ad**************@yahoo.com

__________________________________
Do you Yahoo!?
Yahoo! Finance Tax Center - File online. File on time.
http://taxes.yahoo.com/filing.html

Jul 18 '05 #1
Share this Question
Share on Google+
5 Replies


P: n/a
P
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/fo...onfuse-apache),


I did a very quick fortune file parser as part of:
http://www.pixelbeat.org/rotagator

Pádraig.
Jul 18 '05 #2

P: n/a

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 / ho**@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 /
\--------------------------------------------------------------------/
Jul 18 '05 #3

P: n/a
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/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_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/pyt...er/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
Jul 18 '05 #4

P: n/a
Or I could see (untested code)
import random, sys

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

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

if __name__ == '__main__':
print findfortune (sys.argv[1])
Regards. Mel.
Jul 18 '05 #5

P: n/a
Mel Wilson wrote:
Or I could see (untested code)
import random, sys

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

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

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


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

Jul 18 '05 #6

This discussion thread is closed

Replies have been disabled for this discussion.