How am I doing?

J

Jason

Please don't laugh, this is my FIRST Python script where I haven't
looked at the manual for help...

import string
import random

class hiScores:

hiScores=['10000Alpha','07500Beta','05000Gamma','02500Delta','00000Epsilon']

def showScores(self):
for entry in self.hiScores:
print entry[0:5]," - ",entry[5:]

def addScore(self,score,name):
newScore=string.zfill(score,5)
self.hiScores.append(newScore+name)
self.hiScores.sort(reverse=True)

if len(self.hiScores)==6:
del self.hiScores[-1]

a=hiScores()
print "Original Scores\n---------------"
a.showScores()

while 1:
newScore=random.randint(0,10000)
if string.zfill(newScore,5)>a.hiScores[4][0:5]:
print "Congratulations, you scored %d " % newScore
name=raw_input("Please enter your name :")
a.addScore(newScore,name)
a.showScores()
continue

Anything I could have done differently or any "bad-habits" you think I
have which could lead to ultimate doom I really appreciate to know.

TIA
 
G

George Sakkis

Jason said:
Please don't laugh, this is my FIRST Python script where I haven't
looked at the manual for help...

Sooner or later you should ;)
import string

Don't need it it modern python; use string methods instead.
import random

class hiScores:

The common convention is to use Capitalized words for classes, e.g.
HiScores.
hiScores=['10000Alpha','07500Beta','05000Gamma','02500Delta','00000Epsilon']

hiScores should better be given as parameter when an instance is made,
not hardcoded as a class instance. Also, it is better to separate the
score from the name. Then hiScores can be, say, a list of (score,name)
tuples, e.g. [('10000', 'Alpha'), ('07500', 'Beta'), ..., ('00000',
'Epsilon')]:

def __init__(self, hiScores):
self.hiScores = [(entry[:5], entry[5:]) for entry in hiScores]
def showScores(self):
for entry in self.hiScores:
print entry[0:5]," - ",entry[5:]

If you separate the score from the name in the constructor, you don't
have to split the entries every time showScores is called:
def showScores(self):
for score,name in self.hiScores:
print score, " - ", name

You can express the same more compactly using string interpolation:
def showScores(self):
for entry in self.hiScores:
print "%s - %s" % entry
def addScore(self,score,name):
newScore=string.zfill(score,5)
self.hiScores.append(newScore+name)
self.hiScores.sort(reverse=True)

If you add one entry at a time, it is more efficient to keep the list
sorted and use the bisect.insort function instead of sorting the whole
list:

bisect.insort(self.hiScores, (newScore,name))
if len(self.hiScores)==6:

With your given hiScores, this test is useless; you started with 5
entries and added one so you know there are 6 now. In the more general
case, sort the initial hiScores in the constructor and take the top 5
entries.
del self.hiScores[-1]

You can also remove the last element of a list by self.hiScores.pop()
a=hiScores()
print "Original Scores\n---------------"
a.showScores()

while 1:
newScore=random.randint(0,10000)
if string.zfill(newScore,5)>a.hiScores[4][0:5]:

Two things:
- string.zfill(newScore,5) is better written as newScore.zfill(5)
- a.hiScores[4][0:5] is cryptic; it is better to write a method to give
you the last score so that you can spell it as a.lastScore():

def lastScore(self):
return a.hiScores[-1][0] # assuming (score,name) entries
print "Congratulations, you scored %d " % newScore
name=raw_input("Please enter your name :")
a.addScore(newScore,name)
a.showScores()
continue

"continue" doesn't do anything at the line you put it. When do you want
your program to exit ? As it is, it will loop forever.
Anything I could have done differently or any "bad-habits" you think I
have which could lead to ultimate doom I really appreciate to know.

TIA

HTH,
George
 
J

Jason

George said:
Jason said:
Please don't laugh, this is my FIRST Python script where I haven't
looked at the manual for help...

Sooner or later you should ;)
import string

Don't need it it modern python; use string methods instead.
import random

class hiScores:

The common convention is to use Capitalized words for classes, e.g.
HiScores.
hiScores=['10000Alpha','07500Beta','05000Gamma','02500Delta','00000Epsilon']

hiScores should better be given as parameter when an instance is made,
not hardcoded as a class instance. Also, it is better to separate the
score from the name. Then hiScores can be, say, a list of (score,name)
tuples, e.g. [('10000', 'Alpha'), ('07500', 'Beta'), ..., ('00000',
'Epsilon')]:

def __init__(self, hiScores):
self.hiScores = [(entry[:5], entry[5:]) for entry in hiScores]
def showScores(self):
for entry in self.hiScores:
print entry[0:5]," - ",entry[5:]

If you separate the score from the name in the constructor, you don't
have to split the entries every time showScores is called:
def showScores(self):
for score,name in self.hiScores:
print score, " - ", name

You can express the same more compactly using string interpolation:
def showScores(self):
for entry in self.hiScores:
print "%s - %s" % entry
def addScore(self,score,name):
newScore=string.zfill(score,5)
self.hiScores.append(newScore+name)
self.hiScores.sort(reverse=True)

If you add one entry at a time, it is more efficient to keep the list
sorted and use the bisect.insort function instead of sorting the whole
list:

bisect.insort(self.hiScores, (newScore,name))
if len(self.hiScores)==6:

With your given hiScores, this test is useless; you started with 5
entries and added one so you know there are 6 now. In the more general
case, sort the initial hiScores in the constructor and take the top 5
entries.
del self.hiScores[-1]

You can also remove the last element of a list by self.hiScores.pop()
a=hiScores()
print "Original Scores\n---------------"
a.showScores()

while 1:
newScore=random.randint(0,10000)
if string.zfill(newScore,5)>a.hiScores[4][0:5]:

Two things:
- string.zfill(newScore,5) is better written as newScore.zfill(5)
- a.hiScores[4][0:5] is cryptic; it is better to write a method to give
you the last score so that you can spell it as a.lastScore():

def lastScore(self):
return a.hiScores[-1][0] # assuming (score,name) entries
print "Congratulations, you scored %d " % newScore
name=raw_input("Please enter your name :")
a.addScore(newScore,name)
a.showScores()
continue

"continue" doesn't do anything at the line you put it. When do you want
your program to exit ? As it is, it will loop forever.
Anything I could have done differently or any "bad-habits" you think I
have which could lead to ultimate doom I really appreciate to know.

TIA

HTH,
George
LOL - O dear!!

Well I can certainly see a lot of better methods that you've pointed out
George. For one thing, I was 'wanting' to go down the tuples or
dictionary mode but was under the impression you couldn't sort them
directly. From what I understand, you can't, but the example you have
shown clearly makes sense and a hell-of-alot more practical.

Never knew about the Pop command!!

Thanks again for the help though, really appreciate it.
 
M

Mike Meyer

Jason said:
Please don't laugh, this is my FIRST Python script where I haven't
looked at the manual for help...

import string
import random

class hiScores:
hiScores=['10000Alpha','07500Beta','05000Gamma','02500Delta','00000Epsilon']

def showScores(self):
for entry in self.hiScores:
print entry[0:5]," - ",entry[5:]

def addScore(self,score,name):
newScore=string.zfill(score,5)
self.hiScores.append(newScore+name)
self.hiScores.sort(reverse=True)

if len(self.hiScores)==6:
del self.hiScores[-1]

a=hiScores()
print "Original Scores\n---------------"
a.showScores()

while 1:
newScore=random.randint(0,10000)
if string.zfill(newScore,5)>a.hiScores[4][0:5]:
print "Congratulations, you scored %d " % newScore
name=raw_input("Please enter your name :")
a.addScore(newScore,name)
a.showScores()
continue

Anything I could have done differently or any "bad-habits" you think I
have which could lead to ultimate doom I really appreciate to know.

George already covered a lot of things. I wanted to make a general comment:

The standard idiom is to put all the executable code for a script in a
function, (say "main"), then invoke that function iff your code is run
as a script:

def main():
a = hiScores()
...

if __name__ == "__main__":
main()

That way your program can be used as a module by other applications,
allowing your classes/functions/etc. to be reused by other
applications without having to copy them out of your program.

<mike
 
J

JMH

Thanks for that Mike.

I do remember reading about the __main__ in an online tutorial and
remember thinking "That's handy!".

Totally forgot about that in my own little code so thanks for reminding me.
 
B

Brett Hoerner

Wouldn't the standard idiom be to actually put the code under the
if-name, and not make a whole new main() function?

I'm not sure I see the reason behind main(), couldn't that also
interfere with other modules since main() seems like it might be
common, not sure how it would work as I'm pretty new to Python myself.
from script import * ... what happens when you have two main()s?
 
J

Jason

I've restructured my code with the assistance of George and Mike which
is now as follows...

import random

class HiScores:
def __init__(self,hiScores):
self.hiScores=[(entry[:5],entry[5:]) for entry in hiScores]

def showScores(self):
for name,score in self.hiScores:
print "%s - %s" % name,score

def addScore(self,score,name):
score.zfill(5)
bisect.insort(self.hiScores,(score,name))
if len(self.hiScores)==6:
self.hiScores.pop()

def lastScore(self):
return self.hiScores[-1][0]

def main():

hiScores=[('10000','Alpha'),('07500','Beta'),('05000','Gamma'),('02500','Delta'),('00000','Epsilon')]

a=HiScores(hiScores)
print "Original Scores\n---------------"
a.showScores()

while 1:
newScore=random.randint(0,10000)
if newScore.zfill(5) > a.lastScore():
print "Congratulations, you scored %d " % newScore
name=raw_input("Please enter your name :")
a.addScore(newScore,name)
a.showScores()

if __name__=="__main__":
main()


However doing like the above (which DOES make sense now) raises a few
questions that I've been struggling to find answers for over several
hours now.

1) The most important is that when run, the program crashes with

Traceback (most recent call last):
File "D:\My Documents\Development\Python\highscore1.py", line 35, in
-toplevel-
main()
File "D:\My Documents\Development\Python\highscore1.py", line 28, in main
if newScore.zfill(5) > a.lastScore():
AttributeError: 'int' object has no attribute 'zfill'

I've read as many websites as I can about zfill and I can't see why on
earth it's failing.

2) The output of the predefined hiscores is now...

10000 - Alpha ()
07500 - Beta ()
05000 - Gamma ()
02500 - Delta ()
00000 - Epsilon ()

Why are there the pairing parenthesis there? George very kindly showed
me another way which was to have...

def showScores(self):
for entry in self.hiScores:
print entry[0:5]," - ",entry[5:]

But using that method output the entire list in it's full format (sorry
if that's not the correct terminology). But give me a small plus mark
for changing code and not simply copying George :)

3) The hardest thing to 'understand' is the line...
self.hiScores=[(entry[:5],entry[5:]) for entry in hiScores]

I 'understand' what it's doing, but I don't quite comprehend what the :5
and 5: do. I know that the :5 is technically saying from the start to
position 5, and likewise the 5: would say from position 5 onwards, but I
just can't get my head around how this works.

TIA
 
J

John Hazen

* Jason said:
I've restructured my code with the assistance of George and Mike which
is now as follows...

import random

class HiScores:
def __init__(self,hiScores):
self.hiScores=[(entry[:5],entry[5:]) for entry in hiScores]

With your redefined hiScores, the above is wrong. I think it should
just be:
self.hiScores=[entry for entry in hiScores]

Your original code used the slicing to pull the score and name out of a
single string. Since you've split the string into its two parts, you
don't need the indexing anymore.
def showScores(self):
for name,score in self.hiScores:
print "%s - %s" % name,score

The error you cite below is due to trying to zfill an integer, not a
string. I'm not going to modify your code, but I would use integers all
over for the scores, and only turn to a string (and do the zfill) when
outputting it.

def showScores(self):
for name,score in self.hiScores:
score = str(score).zfill(5) #untested
print "%s - %s" % name,score
def main():

hiScores=[('10000','Alpha'),('07500','Beta'),('05000','Gamma'),('02500','Delta'),('00000','Epsilon')]

This looks like an indentation error to me. Is hiScores indented in
your version?
a=HiScores(hiScores)
print "Original Scores\n---------------"
a.showScores()

while 1:
newScore=random.randint(0,10000)

As I said, I would use int's throughout, but to use it as-is, change the
above line to:
newScore=str(random.randint(0,10000))
if newScore.zfill(5) > a.lastScore():
print "Congratulations, you scored %d " % newScore
name=raw_input("Please enter your name :")
a.addScore(newScore,name)
a.showScores()

if __name__=="__main__":
main()

1) The most important is that when run, the program crashes with

AttributeError: 'int' object has no attribute 'zfill'

I've read as many websites as I can about zfill and I can't see why on
earth it's failing.

I think I explained this above. You're trying to call a string method
on an integer.
2) The output of the predefined hiscores is now...

10000 - Alpha ()
07500 - Beta ()
05000 - Gamma ()
02500 - Delta ()
00000 - Epsilon ()

Are you sure it's not:

('10000', 'Alpha') - ()
etc. ?

I think this is a result of your still doing indexing to separate the
score and the name, even though you've already separated the name and
score into tuples in the predefined list:
hiScores=[('10000','Alpha'),('07500','Beta'),('05000','Gamma'),('02500','Delta'),('00000','Epsilon')]
s=hiScores[0]
s ('10000', 'Alpha')
s[:5] ('10000', 'Alpha')
s[5:] ()
p = (s[:5],s[5:])
p (('10000', 'Alpha'), ())
print "%s - %s" % p ('10000', 'Alpha') - ()

Why are there the pairing parenthesis there? George very kindly showed
me another way which was to have...

def showScores(self):
for entry in self.hiScores:
print entry[0:5]," - ",entry[5:]

But using that method output the entire list in it's full format (sorry
if that's not the correct terminology). But give me a small plus mark
for changing code and not simply copying George :)

3) The hardest thing to 'understand' is the line...
self.hiScores=[(entry[:5],entry[5:]) for entry in hiScores]

This is now slicing into the tuple for each entry, instead of into the
string, so your results are unexpected. slicing past the end of a tuple
returns the empty tuple (which I think is the '()' you're getting in
your output.

HTH-

John
 
A

Andrew Durdin

class HiScores:
def __init__(self,hiScores):
self.hiScores=[(entry[:5],entry[5:]) for entry in hiScores]

In your original code, you were using slicing to extract the first
five digits (being the score) from the string; now that you're using
tuples of (score, name), you don't need the slicing at all. In fact,
the slicing here is not doing what you expect. If we consider a single
instant in this list comprehension, then this is doing the equivalent
of:

entry = ('10000','Alpha')
a = entry[:5] # ('10000', 'Alpha'), as entry only has two items
b = entry[5:] # (), as entry has only two items
new_entry = (a, b)

So the entry in self.hiScores corresponding to this would be a tuple
(('10000', 'Alpha'), ()). When (in showScores()) you use this with %
name, score -- name is the first tuple ('10000', 'Alpha'), and score
is the second (). The % operator takes (always!) a single tuple, and
finds the '10000' and 'Alpha' to match the two "%s"; then score is
just printed afterwards, which is just () here.

Since all you want to do here in __init__() is copy the input, you
could use the following:

self.hiScores=[(entry[0],entry[1]) for entry in hiScores]

Or, using Python's tuple-unpacking, you can get a clearer version:

self.hiScores=[(score, name) for score, name in hiScores]

But since tuples are immutable, it is generally safe to grab another
reference to a tuple instead of copying it; so we can just copy the
whole list:

def __init__(self,hiScores):
self.hiScores=list(hiScores)

I'd also recommend that you use ints, not strings, to store the
scores; when printing the scores, you can use the formatting options
of %d to display the score with the appropriate number of zeroes; or
you could use str(score).zfill(5) if it makes more sense to you (I
haven't given you an example of this change).

% substitution has its own trap, in that the , operator has lower
precedence than %, so whenever you want to use more than one value
with %, you need to wrap them all in parentheses:

def showScores(self):
for name,score in self.hiScores:
print "%s - %s" % (name,score)
def addScore(self,score,name):
score.zfill(5)
bisect.insort(self.hiScores,(score,name))
if len(self.hiScores)==6:
self.hiScores.pop()

If you use ints to store your score, you can remove the line calling
..zfill() here.
You'll need to import the bisect module before you can use it. Note
that bisect.insort() will not work as expected if the list is not in
sorted order. You should make sure the list is sorted in __init__.
def main():

hiScores=[('10000','Alpha'),('07500','Beta'),('05000','Gamma'),('02500','Delta'),('00000','Epsilon')]

a=HiScores(hiScores)
print "Original Scores\n---------------"
a.showScores()

while 1:
newScore=random.randint(0,10000)
if newScore.zfill(5) > a.lastScore():
I've read as many websites as I can about zfill and I can't see why on
earth it's failing.

random.randint() returns an int; zfill() is a method of strings. If
you use ints for the score (as recommended above), then you need only
compare newScore > a.lastScore(); if you want to continue using
strings, then you'll need to use str(newScore) to get a string.

2) The output of the predefined hiscores is now...

Why are there the pairing parenthesis there? George very kindly showed
me another way which was to have...

I've tried to explain why this is occurring above -- it's due to a bug
in __init__(), and another in showScores().
def showScores(self):
for entry in self.hiScores:
print entry[0:5]," - ",entry[5:]

But using that method output the entire list in it's full format (sorry
if that's not the correct terminology). But give me a small plus mark
for changing code and not simply copying George :)

No, you made almost exactly the right change to showScores() needed
because of using tuples to store the name and score. You only tripped
over the % name, score instead of % (name, score) problem that
almost everyone makes (I still do it regularly).

>
3) The hardest thing to 'understand' is the line...
self.hiScores=[(entry[:5],entry[5:]) for entry in hiScores]

I 'understand' what it's doing, but I don't quite comprehend what the :5
and 5: do. I know that the :5 is technically saying from the start to
position 5, and likewise the 5: would say from position 5 onwards, but I
just can't get my head around how this works.

See my explanation of __init__().

Does that help?

Andrew
 
M

Mike Meyer

Brett Hoerner said:
Wouldn't the standard idiom be to actually put the code under the
if-name, and not make a whole new main() function?

Depends on how big the main() function is. By making it a function,
you make it possible for other modules to run it directly. In
particular, if foo.py goes into the python library, then "foo" can be:

#!/usr/bin/env python

from foo import main
main()

which means your main gets the (rather trivial) benefit of being
precompiled.
I'm not sure I see the reason behind main(), couldn't that also
interfere with other modules since main() seems like it might be
common, not sure how it would work as I'm pretty new to Python myself.
from script import * ... what happens when you have two main()s?

You can't have two main()s. You can *define* two main()s, but only one
of them can be bound to that name. So yes, if you do "from foo import *",
then you can lose the binding to one of the mains. But if your code
isn't in a routine, you can't get to it by name anyway. In any case,
"from foo import *" is frowned upon - it makes it hard to figure out
where the variables in the namespace you do that in came from.

<mike
 
D

Dennis Lee Bieber

from script import * ... what happens when you have two main()s?

Except in a very few cases, common wisdom is... Don't Do That...

from x import *

just makes local references to everything inside of x, cluttering up
the namespace, and making things like reload(x) pretty much useless...
--
 
T

Tom Anderson

Wouldn't the standard idiom be to actually put the code under the
if-name, and not make a whole new main() function?

Yes.

The nice thing about the main() function, though, is that you can do the
most basic argument parsing in the if-block. Like so:

def powers(n):
m = 1
while True:
yield m
m = m * n

def main(n, limit):
for power in powers(n):
if (power > limit): break
print power

import sys

if (__name__ == "__main__"):
main(int(sys.argv[1]), int(sys.argv[2]))

That serves as a sort of documentation on the arguments to the script, and
also makes it easier for other scripts to reuse the main logic of the
program, since they don't have to package parameters up as a string array.
It is more verbose, though.
I'm not sure I see the reason behind main(), couldn't that also
interfere with other modules since main() seems like it might be common,
not sure how it would work as I'm pretty new to Python myself.

The two mains would be in different namespaces, so they wouldn't conflict.
from script import *

Don't do that. 'from script import x' is, IMNERHO, bad practice, and 'from
script import *' is exceptionally bad practice. I know a lot of people do,
but that doesn't make it right; namespaces are there for a reason.

tom
 
J

Jason

Tom said:
Wouldn't the standard idiom be to actually put the code under the
if-name, and not make a whole new main() function?

Yes.

The nice thing about the main() function, though, is that you can do the
most basic argument parsing in the if-block. Like so:

def powers(n):
m = 1
while True:
yield m
m = m * n

def main(n, limit):
for power in powers(n):
if (power > limit): break
print power

import sys

if (__name__ == "__main__"):
main(int(sys.argv[1]), int(sys.argv[2]))

That serves as a sort of documentation on the arguments to the script, and
also makes it easier for other scripts to reuse the main logic of the
program, since they don't have to package parameters up as a string array.
It is more verbose, though.
I'm not sure I see the reason behind main(), couldn't that also
interfere with other modules since main() seems like it might be common,
not sure how it would work as I'm pretty new to Python myself.

The two mains would be in different namespaces, so they wouldn't conflict.
from script import *

Don't do that. 'from script import x' is, IMNERHO, bad practice, and 'from
script import *' is exceptionally bad practice. I know a lot of people do,
but that doesn't make it right; namespaces are there for a reason.

tom

I haven't a clue what all this means, but it looks important ! lol

Thanks for the headsup, will take note of what you've said.

Incidentally, at work my main programming platform is VisualStudio .Net,
and I never import the children of namespaces so hopefully this practice
I have will be carried over to Python.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,772
Messages
2,569,593
Members
45,111
Latest member
KetoBurn
Top