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

Ian Kelly ian.g.kelly at gmail.com
Thu Feb 16 01:07:48 CET 2012


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.

Cheers,
Ian



More information about the Python-list mailing list