Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Code Feedback

2 views
Skip to first unread message

mwt

unread,
Feb 6, 2006, 12:33:58 PM2/6/06
to
Hello -
Last night I wrote my first code in Python -- a little
producer/consumer toy to help me begin to understand things. The
"Library" only has a few books. You can choose how many Readers are
trying to check out the books. When no books are available, they have
to wait until some other Reader returns one. Readers are assigned
random reading speeds and wait times between

It runs fine, but I have a few questions about it.
1) Is this good Python code? What should be changed to make it more
Pythonesque?
2) Anybody know how to alter the "while 1: pass" section to make the
app stoppable?
3) The synchronization I'm using works, but is there a better way to do
it?

Thanks for any insight/pointers, etc.

Also, for any newbies like me, feel free to post variations of this
code. In the next version, I'm going to make it so that readers can't
check out the same book twice.

Here's the code:

#!/usr/bin/python
# Filename: Library.py
# author: MWT
# 5 Feb, 2006

import thread
import time
import threading
import random

class Library:
#The goal here is to create a synchronized list of books
def __init__(self):
self.stacks = ["Heart of Darkness", "Die Verwandlung", "Lord of
the Flies", "For Whom the Bell Tolls", "Dubliners", "Cyrano de
Bergerac"]
self.cv = threading.Condition()

def checkOutBook(self):
#remove book from the front of the list, block if no books are
available
self.cv.acquire()
while not len(self.stacks) > 0:
self.cv.wait()
print "waiting for a book..."
bookName = self.stacks.pop(0)
self.cv.release()
return bookName

def returnBook(self, nameOfBook):
#put book at the end of the list, notify that a book is available
self.cv.acquire()
self.stacks.append(nameOfBook)
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.bookName = ""


def run(self):
while 1:
self.bookName = self.library.checkOutBook()
print self.name, "reading", self.bookName
time.sleep(self.readingSpeed)
print self.name, "done reading", self.bookName
self.library.returnBook(self.bookName)
self.bookName = ""
time.sleep(self.timeBetweenBooks)


if __name__=="__main__":

library = Library()
readers = input("Number of Readers?")
for i in range(1,readers):
newReader = Reader(library, "Reader" + str (i),
random.randint(1,7), random.randint(1,7))
newReader.start()
while 1: pass

Dan M

unread,
Feb 6, 2006, 4:37:34 PM2/6/06
to
> 2) Anybody know how to alter the "while 1: pass" section to make the
> app stoppable?

That one I think I can help with! See below.

> while 1: pass

try:
while 1:
pass
except KeyboardInterrupt:
break


Peter Hansen

unread,
Feb 6, 2006, 5:21:29 PM2/6/06
to pytho...@python.org

That might make it "stoppable" (or at least more cleanly stoppable than
it is now) but it doesn't make it efficient.

Adding something like "time.sleep(0.1)" in place of "pass" is
advisable... or the main thread will be "busy-waiting", using up CPU
time, while waiting for Ctrl-C...

-Peter

Jorgen Grahn

unread,
Feb 6, 2006, 6:04:35 PM2/6/06
to
On 6 Feb 2006 09:33:58 -0800, mwt <micha...@gmail.com> wrote:
> Hello -
> Last night I wrote my first code in Python -- a little
> producer/consumer toy to help me begin to understand things. The
> "Library" only has a few books. You can choose how many Readers are
...

> 1) Is this good Python code?

Can't say, but it makes a pretty good reading list[0].

> self.stacks = ["Heart of Darkness", "Die Verwandlung", "Lord of
> the Flies", "For Whom the Bell Tolls", "Dubliners", "Cyrano de
> Bergerac"]

You might want to look into using doc strings properly. You have comments at
the start of functions, but they read more like implementation notes than
"what this method does and why". But I only had a quick look.

/Jorgen
[0] Note to self: pick up some Joseph Conrad later this week.

--
// Jorgen Grahn <grahn@ Ph'nglui mglw'nafh Cthulhu
\X/ snipabacken.dyndns.org> R'lyeh wgah'nagl fhtagn!

snoe

unread,
Feb 6, 2006, 6:40:24 PM2/6/06
to
I believe the while 1: pass is there to keep the main thread alive
until all the readers are done. If you want the program to end after
the readers are done you can append them all to a list then iterate
through and wait for the threads to join()

if __name__=="__main__":

library = Library()
readers = input("Number of Readers?")

readerlist = []


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

readerlist.append(newReader)
for reader in readerlist:
reader.join()

mwt

unread,
Feb 7, 2006, 1:09:11 AM2/7/06
to
Thanks for all the feedback.
Interestingly, I can't seem to get Dan M's code:
[code]

try:
while 1:
pass
except KeyboardInterrupt:
break
[/code]
to work, no matter how many variations I try (including adding in
"time.sleep(0.1)" as Peter Hansen suggested. The program just continues
to execute, ignoring the command to stop. I'm guessing that this has
something to do with the fact that the Reader threads are still
running?

A further question: Can anyone point me to a good description of the
best way to write/use doc strings in Python?

mwt

unread,
Feb 7, 2006, 5:43:41 AM2/7/06
to

Jorgen Grahn wrote:
> You might want to look into using doc strings properly. You have comments at
> the start of functions, but they read more like implementation notes than
> "what this method does and why". But I only had a quick look.

Yeah. They were just my scaffolding notes to myself.

I'd hurry up with that Conrad if I were you, and get on with the
Hemingway. ;)

Sion Arrowsmith

unread,
Feb 7, 2006, 8:15:18 AM2/7/06
to
mwt <micha...@gmail.com> wrote:
>1) Is this good Python code? What should be changed to make it more
>Pythonesque?

> while not len(self.stacks) > 0:

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 .

--
\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

0 new messages