Wanted: Criticism of code for a Python module, plus a Mac tester

HoneyMonster someone at someplace.invalid
Wed Feb 15 20:11:52 EST 2012


On Wed, 15 Feb 2012 17:07:48 -0700, Ian Kelly wrote:

> On Wed, Feb 15, 2012 at 4:33 PM, HoneyMonster
> <someone at someplace.invalid> wrote:
>> Secondly, as a more general point I would welcome comments on code
>> quality, adherence to standards and so forth. The code is at:
> 
> Looks pretty nice overall.  To reduce repetition, I would have
> constructed the CONDITIONS list by iteration like you did the DECK list,
> something like:
> 
> DEALERS = ["Dealer North", "Dealer East", "Dealer South", "Dealer West"]
> VULNERABILITIES = [
>     "Neither vulnerable", "North-South vulnerable",
>     "East-West vulnerable", "Both vulnerable",
> ]
> CONDITIONS = [(d, v) for d in DEALERS for v in VULNERABILITIES]
> 
> You could also eliminate some repetition in the deal method by putting
> the suit lists in a local dict:
> 
>         suits = {'S': self.spades, 'H': self.hearts,
>                  'D': self.diamonds, 'C': self.clubs}
>         for card in cards:
>             suits[DECK[card][SUIT]].append(DECK[card][RANK])
> 
> Another comment I had at first was that instead of creating the pack
> list, which is just a list of indexes into DECK, you could shuffle and
> deal the DECK list directly.  But then I realized that the indirection
> allows you to easily sort the cards in the desired order, so that should
> be left alone.

Thank you very much indeed, Ian.

Your second suggestion has opened my eyes; it had not even occurred to me 
that the "value" element of a dictionary entry could be a list. Just 
shows me how much I have to learn! Isn't Python great?

As to your first suggestion though, I am having some difficulty. Note 
that the vulnerability rotates; i.e. CONDITIONS[4] is not the same as 
CONDITIONS[0].
Is there a better way of doing it than a simple list.append()?

Thanks again.



More information about the Python-list mailing list