George Sakkis wrote:
> "Jason" <[EMAIL PROTECTED]> 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
> 
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.

-- 
http://mail.python.org/mailman/listinfo/python-list

Reply via email to