How am I doing?
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=['1Alpha','07500Beta','05000Gamma','02500Delta','0Epsilon'] 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,1) 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 -- http://mail.python.org/mailman/listinfo/python-list
Re: How am I doing?
"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=['1Alpha','07500Beta','05000Gamma','02500Delta','0Epsilon'] 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. [('1', 'Alpha'), ('07500', 'Beta'), ..., ('0', '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,1) > 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 -- http://mail.python.org/mailman/listinfo/python-list
Re: How am I doing?
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=['1Alpha','07500Beta','05000Gamma','02500Delta','0Epsilon'] > > 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. [('1', 'Alpha'), ('07500', 'Beta'), ..., ('0', > '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,1) >> 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
Re: How am I doing?
Jason <[EMAIL PROTECTED]> writes: > 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=['1Alpha','07500Beta','05000Gamma','02500Delta','0Epsilon'] > > 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,1) > 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. http://www.mired.org/home/mwm/ Independent WWW/Perforce/FreeBSD/Unix consultant, email for more information. -- http://mail.python.org/mailman/listinfo/python-list
Re: How am I doing?
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. -- http://mail.python.org/mailman/listinfo/python-list
Re: How am I doing?
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? -- http://mail.python.org/mailman/listinfo/python-list
Re: How am I doing?
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=[('1','Alpha'),('07500','Beta'),('05000','Gamma'),('02500','Delta'),('0','Epsilon')] a=HiScores(hiScores) print "Original Scores\n---" a.showScores() while 1: newScore=random.randint(0,1) 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... 1 - Alpha () 07500 - Beta () 05000 - Gamma () 02500 - Delta () 0 - 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 -- http://mail.python.org/mailman/listinfo/python-list
Re: How am I doing?
* Jason <[EMAIL PROTECTED]> [2005-09-19 16:28]: > 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=[('1','Alpha'),('07500','Beta'),('05000','Gamma'),('02500','Delta'),('0','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,1) 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,1)) > 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... > > 1 - Alpha () > 07500 - Beta () > 05000 - Gamma () > 02500 - Delta () > 0 - Epsilon () Are you sure it's not: ('1', '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=[('1','Alpha'),('07500','Beta'),('05000','Gamma'),('02500','Delta'),('0','Epsilon')] >>> s=hiScores[0] >>> s ('1', 'Alpha') >>> s[:5] ('1', 'Alpha') >>> s[5:] () >>> p = (s[:5],s[5:]) >>> p (('1', 'Alpha'), ()) >>> print "%s - %s" % p ('1', '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. > TIA HTH- John -- http://mail.python.org/mailman/listinfo/python-list
Re: How am I doing?
On 9/20/05, Jason <[EMAIL PROTECTED]> wrote: > 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 = ('1','Alpha') a = entry[:5]# ('1', '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 (('1', 'Alpha'), ()). When (in showScores()) you use this with % name, score -- name is the first tuple ('1', 'Alpha'), and score is the second (). The % operator takes (always!) a single tuple, and finds the '1' 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=[('1','Alpha'),('07500','Beta'),('05000','Gamma'),('02500','Delta'),('0','Epsilon')] > > a=HiScores(hiScores) > print "Original Scores\n---" > a.showScores() > > while 1: > newScore=random.randint(0,1) > 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 -- http://mail.python.org/mailman/listinfo/python-list
Re: How am I doing?
"Brett Hoerner" <[EMAIL PROTECTED]> writes: > 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. http://www.mired.org/home/mwm/ Independent WWW/Perforce/FreeBSD/Unix consultant, email for more information. -- http://mail.python.org/mailman/listinfo/python-list
Re: How am I doing?
On Mon, 19 Sep 2005, Brett Hoerner wrote: > 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 -- Science runs with us, making us Gods. -- http://mail.python.org/mailman/listinfo/python-list
Re: How am I doing?
Tom Anderson wrote: > On Mon, 19 Sep 2005, Brett Hoerner wrote: > >> 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. -- http://mail.python.org/mailman/listinfo/python-list