----- Original Message ----- > From: "C Smith" <illusiontechniq...@gmail.com> > To: python-list@python.org > Sent: Tuesday, 4 November, 2014 4:28:33 PM > Subject: Code review > > I was wondering if I could get some feedback on the biggest thing I > have done as an amateur Python coder. The sidepots algorithm isn't > correct yet, but I haven't worked on it in a while and thought I > would > get some advice before diving back in. > > import random, os > ################################################################## > class Table(object): > def __init__(self,bigblind=20,PLAYERNUM=0,pot=0,PLAYERORDER=None, > hand_done=0,\ > > left_to_act=None,cost_to_play=0,in_hand=None,last_raise=0,cards_in_play=None,round='preflop'): > if cards_in_play is None: > cards_in_play = [] > if in_hand is None: > in_hand = [] > if PLAYERORDER is None: > PLAYERORDER = [] > if left_to_act is None: > left_to_act = []
[snip hundreds of code lines] - Most methods have too much code, split methods into more unitary logical functions. - For instance, add classes 'Hand' and 'Card' - not enough comments - have you written some tests? The code reached a reasonable size where tests would be very helpful (module unittest) - why do you use uppercase attributes ? like PLAYERORDER. These are not 'constants', you change them Example of (incomplete) Card and Hand implementation (python 2.7): class Card(object): """Card implementation.""" def __init__(self, suit, rank): self._suit = suit self._rank = rank # write test card1 == card2 def __eq__(self, other): return (self.suit, self.rank) == (other.suit, other.rank) # use cards as dict keys def __hash__(self): return hash(self.suit, self.rank) # will allow python to sort a list of cards def __lt__(self, other): return int(self) < int(other) def __int__(self): """Return the numerical value of a card""" # the dictionary is incomplete return {'1':1,'2':2,'J':11,'Q':12}[self.rank] def __str__(self): return '<Card(%s of %s)>' % (self.rank, self.suit) # alias in case card.rank has more meaning than int(card) @property def rank(self): return int(self) # read only access to suit @property def suit(self): return self._suit class Hand(object): """Short incomplete example of Hand class""" def __init__(self): self._cards = [] def add(self, card): if not self.full: self._cards.append(card) if card.rank == 14: # it's an Ace # trick to get Ace considered as 1 as well self._cards.append(Card(card.suite, '1')) # generate the ordered sequence of cards, hiding the '1' cards @property def cards(self): return (card for card in sorted(self._cards) if card.rank > 1) # allow to write len(hand) def __len__(self): return len(list(self.cards)) #property def full(self): return len(self) == 5 # allow to write 'for card in hand:' def __iter__(self): return iter(self.cards) def remove(self, card): # and so on... Otherwise: replace if left_to_act is None: left_to_act = [] self.left_to_act = left_to_act by self.left_to_act = left_to_act or [] replace if ranky == 9 or ranky == 5 by if ranky in [9,5] replace if foo == True by if foo replace if len(self.left_to_act) == 0 by if self.left_to_act And much more... but honestly, there's too much code :) I'll let you chew on this one. JM -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -- https://mail.python.org/mailman/listinfo/python-list