450 Pound Library Program

[mwt]

So in a further attempt to learn some Python, I've taken the little
Library program
(http://groups.google.com/group/comp....a9ccf1bc136f84)
I wrote and added several features to it. Readers now quit when they've
read all the books in the Library. Books know how many times they've
been read. Best of all, you can now create your own list of books to
read!

Again, the point of all this is to get used to programming in Python.
So although the program is trivial, any feedback on style, structure,
etc. would be much appreciated. I'm a convert from Java, so I've
probably got some unconscious Javanese in there somewhere. Help me get
rid of it!

Here's the new, improved program:

#!/usr/bin/python
# Filename: Library.py
# author: mwt
# Feb, 2006
 
import thread
import time
import threading
import random
 
 
 
class Library2:
def __init__(self, listOfBooks, totalBooks):
self.stacks = listOfBooks
self.cv = threading.Condition()
self.totalBooks = totalBooks
 
def checkOutBook(self, readerName):
"'Remove book from the front of the list, block if no books are
available'"
self.cv.acquire()
while len(self.stacks) == 0:
self.cv.wait()
print "%s waiting for a book..." %readerName
book = self.stacks.pop(0)
self.cv.release()
return book
 
def returnBook(self, returnedBook):
"'put book at the end of the list, notify that a book is
available'"
returnedBook.wasRead()
self.cv.acquire()
self.stacks.append(returnedBook)
self.cv.notify()
self.cv.release()
 
class Reader(threading.Thread):
 
def __init__(self, library, name, readingSpeed, timeBetweenBooks):
threading.Thread.__init__(self)
self.library = library
self.name = name
self.readingSpeed = readingSpeed
self.timeBetweenBooks = timeBetweenBooks
self.book = ""
self.numberOfBooksRead = 0
 
def run(self):
"'Keep checking out and reading books until you've read all in
the Library'"
while  self.numberOfBooksRead < self.library.totalBooks:
self.book = self.library.checkOutBook(self.name)
print "%s reading %s" %(self.name, self.book.title),
time.sleep(self.readingSpeed)
self.numberOfBooksRead += 1
self.library.returnBook(self.book)
print "%s done reading %s" %(self.name, self.book.title),
print"Number of books %s has read: %d" %(self.name,
self.numberOfBooksRead)
self.bookName = ""
time.sleep(self.timeBetweenBooks)
print "%s done reading." %self.name
 
class Book:
def __init__(self, author, title):
self.author = author
self.title = title
self.numberOfTimesRead = 0
#print "%s,%s" % (self.author, self.title),#print as books are
loaded in
 
def wasRead(self):
self.numberOfTimesRead += 1
print "Number of times %s has been read: %d" %(self.title,
self.numberOfTimesRead)
 
if __name__=="__main__":
 
print "\nWELCOME TO THE THURMOND STREET PUBLIC LIBRARY"
print "Checking which books are avialable...\n"
try:
theBookFile = open("books.txt", "r")#Create your own list of
books!
stacks = []#a place to put the books
for line in theBookFile.readlines():
L = line.split (",")  # a comma-delimited list
author = L[0]
bookName =  L[1]
newBook = Book(author, bookName)
stacks.append(newBook)
theBookFile.close()
except IOError:
print "File not found!"
#string = "How many books would you like in the Library?[1-" +
str(len(stacks)) + "]"
totalBooks = input("How many books would you like in the
Library?[1-" + str(len(stacks)) + "]")
stacks[totalBooks: len(stacks)] = []
print "Number of books in the Library is:", len(stacks)
library = Library2(stacks, totalBooks)
readers = input("\nHow many readers would you like?")
print "Number of readers is:", readers, "\n"
for i in range(0,readers):
newReader = Reader(library, "Reader" + str (i),
random.randint(1,7), random.randint(1,7))
newReader.start()
 

And here's a handy text file of books for you, so you don't have to
make your own:

Conrad, Heart of Darkness
Kafka, Die Verwandlung
Hemingway, For Whom the Bell Tolls
James Joyce, Dubliners
Moliere, Cyrano de Bergerac
William Golding, Lord of the Flies
Dostoevski, Crime and Punishment
Cervantes, Don Quixote
Camus, L'Etranger
Tolstoy, War and Peace
Poe, Tales
Faulkner, The Sound and the Fury
Orwell, 1984
Fitzgerald, The Great Gatsby
Steinbeck, The Grapes of Wrath
Huxley, Brave New World
Twain, The Adventures of Huckleberry Finn
Mann, Der Tod in Venedig
Kesey, Sometimes a Great Notion
Pynchon, Gravity's Rainbow
McEwan, The Cement Garden
Márquez, Cien Años de Soledad
Salinger, The Catcher in the Rye
Miltion, Paradise Lost
Chapman et al , The Pythons
 

[Magnus Lycka]

just a few style notes...

def checkOutBook(self, readerName):
"'Remove book from the front of the list, block if no books are
available'"
I don't understand what "' is supposed to imply. If you
meant to use triple quoting, you need to use ''' or """.
Then the string can contain line breaks. (Perhaps you have
a really stupid email client/news client/editor/whatever that
replaces ''' with "'? Please loose that or use """.)

Also, please use lines that are short enough not to wrap
around. Line wrapping makes the code very ugly. Do like this:

def checkOutBook(self, readerName):
"""Remove book from the front of the list, block if no
books are available."""

[...] for line in theBookFile.readlines():
In modern Python you simply write:
for line in theBookFile:
L = line.split (",") # a comma-delimited list
author = L[0]
bookName = L[1]
Why bother with L? The follwing is as clear I think, and solves
the problem of commas in the title. Also, don't put a space between
the callable and the parenthesis please. See the Python style guide,
PEP 008.

author, bookName = line.split(",", 2)

[...] totalBooks = input("How many books would you like in the
Library?[1-" + str(len(stacks)) + "]")

Again, don't make the lines so long. You don't have to do that.
You can break lines freely inside (), {} and [], and adjacent
string literals are automatically concatenated.

totalBooks = input("How many books would you like in "
"the Library?[1-%d]" % len(stacks))

[bruno at modulix]

mwt wrote:

So in a further attempt to learn some Python, I've taken the little
Library program
(http://groups.google.com/group/comp....a9ccf1bc136f84)
I wrote and added several features to it. Readers now quit when they've
read all the books in the Library. Books know how many times they've
been read. Best of all, you can now create your own list of books to
read!

Again, the point of all this is to get used to programming in Python.
So although the program is trivial, any feedback on style, structure,
etc. would be much appreciated. I'm a convert from Java,
Welcome !-)
so I've
probably got some unconscious Javanese in there somewhere. Help me get
rid of it!
Well, apart from namingConventions (vs naming_conventions), I did not
spot too much javaism in your code.
Here's the new, improved program:
[code]
#!/usr/bin/python
# Filename: Library.py
# author: mwt
# Feb, 2006

import thread
import time
import threading
import random

class Library2:
Old-style classes are deprecated, use new-style classes instead:
class Library2(object):
def __init__(self, listOfBooks, totalBooks):
self.stacks = listOfBooks
If its a collection of books, why not call it 'books' ?
self.cv = threading.Condition()
self.totalBooks = totalBooks
What is 'totalBooks' ?
def checkOutBook(self, readerName):
"'Remove book from the front of the list, block if no books are
available'"
Why not using triple-quoted strings ?
self.cv.acquire()
while len(self.stacks) == 0:
self.cv.wait()
print "%s waiting for a book..." %readerName
book = self.stacks.pop(0)
This last line will crash (IndexError) on an empty list, and then the
resource may not be released... A first step would be to enclose this in
a try/finally block.
self.cv.release()
return book

def returnBook(self, returnedBook):
"'put book at the end of the list, notify that a book is
available'"
returnedBook.wasRead()
self.cv.acquire()
self.stacks.append(returnedBook)
self.cv.notify()
self.cv.release()
You have a recurring pattern in the last 2 methods: aquire/do
something/release. You could factor this out in a method decorator
(don't forget that functions and methods are objects too, so you can do
a *lot* of things with them).
class Reader(threading.Thread):

def __init__(self, library, name, readingSpeed, timeBetweenBooks):
threading.Thread.__init__(self) or :
super(Reader, self).__init__()

but this wouldn't make a big difference here
self.library = library
self.name = name
self.readingSpeed = readingSpeed
self.timeBetweenBooks = timeBetweenBooks
self.book = ""
You later user Reader.book to hold a reference to a Book object, so it
would be wiser to initialize it with None.
self.numberOfBooksRead = 0

def run(self):
"'Keep checking out and reading books until you've read all in
the Library'"
while self.numberOfBooksRead < self.library.totalBooks:
self.book = self.library.checkOutBook(self.name)
print "%s reading %s" %(self.name, self.book.title),
time.sleep(self.readingSpeed)
self.numberOfBooksRead += 1
self.library.returnBook(self.book)
print "%s done reading %s" %(self.name, self.book.title),
print"Number of books %s has read: %d" %(self.name,
self.numberOfBooksRead)
self.bookName = ""
time.sleep(self.timeBetweenBooks)
print "%s done reading." %self.name

class Book:
def __init__(self, author, title):
self.author = author
self.title = title
self.numberOfTimesRead = 0
#print "%s,%s" % (self.author, self.title),#print as books are
loaded in

def wasRead(self):
self.numberOfTimesRead += 1
print "Number of times %s has been read: %d" %(self.title,
self.numberOfTimesRead)

if __name__=="__main__":

You should define a main() function and call it from here.
print "\nWELCOME TO THE THURMOND STREET PUBLIC LIBRARY"
print "Checking which books are avialable...\n" s/avialable/available/ !-)
try:
theBookFile = open("books.txt", "r")#Create your own list of
books!
Filenames should not be hardcoded.

(Ok, you probably know this already, but I thought it would be better to
point this out for newbie programmers)
stacks = []#a place to put the books
for line in theBookFile.readlines():
L = line.split (",") # a comma-delimited list
Mmm... may not be the best format. What if there are commas in the book
title ? Hint : there's a CSV module in the standard lib.

Also, by convention, UPPERCASE identifiers are considered as CONSTANTS
author = L[0]
bookName = L[1]
newBook = Book(author, bookName)
stacks.append(newBook)
You can smash all this in a single line:
stacks = [Book(line.split(",", 1) for line in theBookFile]
theBookFile.close()
except IOError:
print "File not found!"
An IOError can result from a lot of different reasons (like user not
having read access on the file). So don't assume the file was not found:

except IOError, e:
print "Could not open file %s : %s" % ('books.txt', e)

You may also want to exit here...
#string = "How many books would you like in the Library?[1-" +
str(len(stacks)) + "]"
totalBooks = input("How many books would you like in the
Library?[1-" + str(len(stacks)) + "]")
1/ use raw_input() and *validate* user data

2/ use string formatting:
prompt = "How many books would you like in the "
" Library? [1-%d]" % len(stacks)
stacks[totalBooks: len(stacks)] = [] stacks = stacks[:totalBooks]

May be less efficient, but much more readable IMHO.

Also, the naming is somewhat misleading. Something along the line of
numberOfBooks or booksCount would probably be better.
print "Number of books in the Library is:", len(stacks)
You've called len(stacks) 3 times in 3 lines.
library = Library2(stacks, totalBooks)
What's the use of totalBooks here ? The Library2 class can figure it out
by itself, so it's useless - and it goes against DRY and SPOT.

hint: given the use of Library.totalBook, you'd better make it a
computed attribute

class LibraryN(object):
booksCount = property(fget=lambda self: len(self.books))
readers = input("\nHow many readers would you like?")
idem. Also, something like readersCount would be less misleading (I
first thought it was a collection of Reader objects)
print "Number of readers is:", readers, "\n"
for i in range(0,readers):
newReader = Reader(library, "Reader" + str (i),
random.randint(1,7), random.randint(1,7))
performance hint: names are first looked up in the local namespace:

randint = random.randint
for i in range(0, readers):
newReader = Reader(library,
"Reader%d" % i,
randint(1,7),
randint(1,7))
newReader.start()

My 2 cents...

(snip)

--
bruno desthuilliers
python -c "print '@'.join(['.'.join([w[::-1] for w in p.split('.')]) for
p in 'o****@xiludom.gro'.split('@')])"

[bruno at modulix]

Magnus Lycka wrote:

just a few style notes...
(snip)
Why bother with L? The follwing is as clear I think, and solves
the problem of commas in the title. Also, don't put a space between
the callable and the parenthesis please. See the Python style guide,
PEP 008.

author, bookName = line.split(",", 2)

"toto, tata, tutu".split(",", 2) ['toto', ' tata', ' tutu']

You want line.split(',', 1) here.

--
bruno desthuilliers
python -c "print '@'.join(['.'.join([w[::-1] for w in p.split('.')]) for
p in 'o****@xiludom.gro'.split('@')])"

[Sion Arrowsmith]

mwt <mi*********@gmail.com> wrote:

while len(self.stacks) == 0:

To (kind of) repeat myself, the idiomatic Python would be:

while not self.stacks:

An empty list is considered to be false, hence testing the list
itself is the same as testing len(l) > 0 .

As someone else has noticed, you're using len() an awful lot
when you don't need to.

--
\S -- si***@chiark.greenend.org.uk -- http://www.chaos.org.uk/~sion/
___ | "Frankly I have no feelings towards penguins one way or the other"
\X/ | -- Arthur C. Clarke
her nu becomeþ se bera eadward ofdun hlæddre heafdes bæce bump bump bump

[plahey]

Ok,

I give up. DRY = Don't Repeat Yourself (google Pragmatic Programmers)
but SPOT? Google is little help here, SPOT is too common a word.

Thanks!

[Magnus Lycka]

pl****@alumni.caltech.edu wrote:

Ok,

I give up. DRY = Don't Repeat Yourself (google Pragmatic Programmers)
but SPOT? Google is little help here, SPOT is too common a word.

The only SPOT I worked with (as I know of) was SPOT4
(Le Systeme Pour 'l Observation de la Terre) but that's
probably not it...

Single Point Of Truth? It seems some claim that DRY and
SPOT are the same things.
http://www.artima.com/cppsource/reducepnp3.html

[bruno at modulix]

pl****@alumni.caltech.edu wrote:

Ok,

I give up. DRY = Don't Repeat Yourself (google Pragmatic Programmers)
but SPOT? Google is little help here, SPOT is too common a word.

Single Point Of Truth (or Single Point Of Transformation)

And before you mention it, yes, it's *almost* the same thing as DRY !-)
--
bruno desthuilliers
python -c "print '@'.join(['.'.join([w[::-1] for w in p.split('.')]) for
p in 'o****@xiludom.gro'.split('@')])"

[bruno at modulix]

Magnus Lycka wrote:

pl****@alumni.caltech.edu wrote:

Ok,

I give up. DRY = Don't Repeat Yourself (google Pragmatic Programmers)
but SPOT? Google is little help here, SPOT is too common a word.

The only SPOT I worked with (as I know of) was SPOT4
(Le Systeme Pour 'l Observation de la Terre) but that's
probably not it...

Probably not !-)
Single Point Of Truth?
And the winner is... <your-name-here />
It seems some claim that DRY and
SPOT are the same things.
http://www.artima.com/cppsource/reducepnp3.html

Well, they are at least very closely related !-)

And I of course forgot OnceAndOnlyOnce...

--
bruno desthuilliers
python -c "print '@'.join(['.'.join([w[::-1] for w in p.split('.')]) for
p in 'o****@xiludom.gro'.split('@')])"

[mwt]

A million thanks for in-depth critique. I look forward to figuring out
half of what you're talking about ;)

[mwt]

BTW - I can't find any code examples of how to interrupt a beast like
this. A regular ctrl-c doesn't work, since the separate threads just
keep executing. Is this a case where you need to iterate through all
threads and stop each one individually? How would that look?