How am I doing?

George Sakkis gsakkis at rutgers.edu
Sun Sep 18 21:24:10 EDT 2005


"Jason" <jason at jasonmhirst.co.uk> wrote:

> 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




More information about the Python-list mailing list