[Tutor] First code snipet
davea at ieee.org
Sun Jul 26 13:49:36 CEST 2009
Darth Kaboda wrote:
>> Subject: Re: [Tutor] First code snipet
>> From: andreas at kostyrka.org
>> To: darthkaboda at msn.com
>> CC: tutor at python.org
>> Date: Sun, 26 Jul 2009 03:02:27 +0200
>> Some things:
>> 1) It's Python, not Phython.
>> 2) Slightly shorter solution to your problem:
>> import random
>> input = range(53)
>> print input
>> 3) The important part here is that reimplementing something that is in
>> the standard library does not make sense usually.
>> 4) A more sensible implementation of shuffle comes directly out of
>> def shuffle(self, x, random=None, int=int):
>> """x, random=random.random -> shuffle list x in place; return
>> Optional arg random is a 0-argument function returning a random
>> float in [0.0, 1.0); by default, the standard random.random.
>> if random is None:
>> random = self.random
>> for i in reversed(xrange(1, len(x))):
>> # pick an element in x[:i+1] with which to exchange x[i]
>> j = int(random() * (i+1))
>> x[i], x[j] = x[j], x[i]
>> Notice that it is not exactly optimal, one could easily replace the
>> reversed(xrange()) expression with a 3 parameter version of xrange:
>> for i in xrange(len(x) - 1, 0, -1):
>> Am Samstag, den 25.07.2009, 17:15 -0700 schrieb Darth Kaboda:
>>> I'm starting to learn Python as it seems to be adopted by many
>>> companies that I've been looking to apply to. I used the book Learning
>>> Phython to get the basics of the language and many of the gotcha's. I
>>> think I know enough of the basic features of the language to start
>>> playing around and can understand most code I've seen so far by
>>> looking at some of the posts here.
>>> The one worry I have is not coding things the Phython way as I've been
>>> a Visual Basic programmer for so long and a C++ programmer before
>>> that. So would like to have people look at a simplistic class
>>> (shuffles lists of objects wrote it for shuffling cards but
>>> with Phython could be used for any "list" type object) I wrote to give
>>> me some feedback on the style of it.
>>> Any feedback is appreciated.
>>> Tutor maillist - Tutor at python.org
> Thanks for the correction not sure why I typed it that way after typing it correctly in the first sentence, must have wondered off somewhere in my mind I guess.
> Oops I guess I should have also mentioned what I was trying to accomplish with this class. I wanted to mimic how a deck of cards get's shuffled in the real world. i.e. split the deck and shuffle the two halves together.
Good point. It is important to state your goal for the code, preferably
in comments in your code, as well as in the message asking us the
question. Suggest you include a doc-string for each class and method
But it's important to tell us how you expect to use it, as well. If
it's for your own learning, then it's pretty good as it is. But if you
intend to show it to prospective employers, there are a lot of ways to
clean it up.
Unlike Java, not everything needs to be in a class. This shuffler class
accomplishes nothing that the module doesn't already do. The only
instance attribute you use is rgen, and that could just as readily have
been a global (visible within the entire module).
Naming is important. I think I would have factored it into three
functions cut() and ripple(). shuffle() simply calls cut(), then ripple().
You need to know the library better. Andreas already mentioned the std
library random.shuffle(), and you correctly explained why you didn't use
it. But there are other library functions that would make your code
more concise, more readable, and maybe faster.
Starting with shuffle() (which I'd call ripple())
Because Python makes it easy to swap two variables in-place, it's common
to swap arguments directly, rather than copy them to local attributes.
So I'd replace the first 4 lines with:
if self.rgen.randint(0,1): #randomly decide which pile is shuffled
inpile2, inpile1 = inpile1, inpile2
len() is very fast, so there's no need for a separate variable to keep
track of the lengths of inpile1 and inpile2. So eliminate all refs to
pile1c and pile2. In the while, just use
while len(inpile1) and len(inpile2):
Lists have no type associated with them. Each object in a list has its
own type. So there's no meaning to constructing an empty list out of
rpile = 
pop(0) is slow, compared with pop(-1). If you used the latter, you
would have to do a reverse on the list when you were done to keep the
realism. And that would make the code less readable. So I think this
is an unnecessary optimization. But keep it in mind if you're ever
dealing with thousands of items in a list. Peeling them off from the
front one at a time will be slow.
The logic of popping a random number of cards from one list, and
appending them to the result could be done the same way you do the
cut(), using slices. I think it would be worth it if you made a
separate function out of it, to call from whichever deck was being
Where you do the if i < pile1c you could instead use the min()
i = min(len(inpile1), self.rgen.randint(1,4))
At the end of the function, the if pile1c: elif ... logic could be
as there's no harm in extending with an empty list.
More information about the Tutor