Path: csiph.com!x330-a1.tempe.blueboxinc.net!usenet.pasdenom.info!gegeweb.42!gegeweb.eu!nntpfeed.proxad.net!proxad.net!feeder1-2.proxad.net!46.17.56.130.MISMATCH!rt.uk.eu.org!newsfeed.xs4all.nl!newsfeed6.news.xs4all.nl!xs4all!post.news.xs4all.nl!not-for-mail Return-Path: X-Original-To: python-list@python.org Delivered-To: python-list@mail.python.org X-Spam-Status: OK 0.022 X-Spam-Evidence: '*H*': 0.96; '*S*': 0.00; 'subject:module': 0.04; 'subject:Python': 0.05; 'forth.': 0.07; 'subject:code': 0.07; 'indexes': 0.09; 'received:mail-lpp01m010-f46.google.com': 0.09; 'constructed': 0.16; 'deck': 0.16; 'deck,': 0.16; 'repetition': 0.16; 'repetition,': 0.16; 'subject:Mac': 0.16; 'wed,': 0.17; 'wrote:': 0.18; 'cheers,': 0.20; 'header:In-Reply-To:1': 0.22; 'feb': 0.22; 'creating': 0.25; 'code': 0.26; 'looks': 0.27; 'putting': 0.28; 'lists': 0.28; 'message-id:@mail.gmail.com': 0.29; 'pm,': 0.29; 'pretty': 0.31; 'received:209.85.215.46': 0.32; 'list': 0.32; 'sort': 0.33; 'instead': 0.33; 'to:addr:python- list': 0.35; 'something': 0.35; 'standards': 0.35; 'comment': 0.36; 'list,': 0.36; 'but': 0.37; 'received:google.com': 0.37; 'another': 0.37; 'received:209.85': 0.38; 'suit': 0.38; 'allows': 0.38; 'could': 0.38; 'some': 0.38; 'should': 0.38; 'received:209.85.215': 0.39; 'received:209': 0.39; 'point': 0.40; 'to:addr:python.org': 0.40; 'skip:s 40': 0.40; 'more': 0.61; 'eliminate': 0.63; 'quality,': 0.63; 'card': 0.65; 'cards': 0.66; 'directly.': 0.68; 'order,': 0.73; 'realized': 0.73; 'dealers': 0.84; 'subject:plus': 0.91; 'alone.': 0.93 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :content-type; bh=cwiVcGdyb8wrcFitCo1Csp9owPMY2b40nlCdScXhqVo=; b=GGpg+4XHaztzbQBGAcXAtZ43fDJCoKtk8BSU41/9rrYG2BJ1j6tnddiM2EYZaCNqoA BgKNSu5AQfUTmWjoip+0Jk5x44hJMVc7dKCqbbqDySux7pxoj0P2DLKqPMk32CLS124I X2nQBbXAeMYc8HY1ZnnrCikCznjEvOuAVcuuU= MIME-Version: 1.0 In-Reply-To: References: From: Ian Kelly Date: Wed, 15 Feb 2012 17:07:48 -0700 Subject: Re: Wanted: Criticism of code for a Python module, plus a Mac tester To: python-list@python.org Content-Type: text/plain; charset=ISO-8859-1 X-BeenThere: python-list@python.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: General discussion list for the Python programming language List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Newsgroups: comp.lang.python Message-ID: Lines: 31 NNTP-Posting-Host: 2001:888:2000:d::a6 X-Trace: 1329350900 news.xs4all.nl 6974 [2001:888:2000:d::a6]:43858 X-Complaints-To: abuse@xs4all.nl Xref: x330-a1.tempe.blueboxinc.net comp.lang.python:20492 On Wed, Feb 15, 2012 at 4:33 PM, HoneyMonster 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