[Tutor] First code snipet

[Dave Angel]

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)
>> random.shuffle(input)
>> 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
>> random.py:
>>
>> def shuffle(self, x, random=None, int=int):
>> """x, random=random.random -> shuffle list x in place; return
>> None.
>>
>> 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):
>>
>> Andreas
>>
>>
>>
>> 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.
>>>
>>> Thanks,
>>> Brian
>>> _______________________________________________
>>> Tutor maillist - Tutor at python.org
>>> http://mail.python.org/mailman/listinfo/tutor
>>>       
>
>  
>
> 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. 
>
>  
>
> Brian
>
>
>  
>
>   
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 
declaration.

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.


So...

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 
down first
            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 
another one.
     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 
operated on.

Where you do the    if i < pile1c     you could instead use the min() 
function.
    i = min(len(inpile1), self.rgen.randint(1,4))

At the end of the function, the  if pile1c:  elif ...  logic could be 
replaced by
     rpile.extend(inpile1).extend(inpile2)

as there's no harm in extending with an empty list.

HTH,   DaveA