Chinese Poker using classes

I developed a Chinese Poker game for better understanding classes in Python. It is not finished yet, but the main part is.

The game mechanics:

-
There is 1 board for each player. Each board have 3 rows.

-
The first row can receive up to 3 cards, and the other 2, 5 cards each. A total of 13 cards in each board

-
First, each player receive 5 cards and then place then within the boards. Player1 place all his cards, and then Player2 place all his cards.

-
After that, each player receive 3 cards per turn. Place 2 cards in the boards, and discard the last one. Cards are dealed until the board is full.

My code executes them and the score system is the next step. I'd like you to evaluate my code and point suggestions to improve it.

```
from random import shuffle

def turn_str(n):
return '%d' % n

deck = (map(turn_str, range(2,11)) + ['J', 'Q', 'K', 'A']) * 4
deck.sort()

naipes = ['c', 'h', 's', 'd'] * 13

def add_suits(x, y):
return x + y

full_deck = map(add_suits, deck, naipes)
shuffle(full_deck)

class User(object):
def __init__ (self, name):
self.name = name
self.board = [[],[],[]]
self.hand = []

def receive_cards(self, number_of_cards):
self.hand = full_deck[0:number_of_cards]
del full_deck[0:number_of_cards]

def board_limit(self, board_number):
if board_number == 1:
if len(self.board[board_number - 1]) < 3:
return True
else:
return False
if board_number == 2 or board_number == 3:
if len(self.board[board_number - 1]) < 5:
return True
else:
return False

def place_first_cards(self):
print "Your time to play %s. Here is your hand. Pick a card and choose a row to place it" % self.name
while len(self.hand) != 0:
print '-'.join(self.hand)
for i in self.board:
print i
card_placed = int(raw_input("Choose a

Repeat actions

Look at the bottom of your code. This is the layout:

do something with user1
do the same thing with user2

do something with user1
do the same thing with user2

...

You have a lot of repeated code down there. To reduce this repetition, you should store the two players in an array, and then each time loop through the array an do what you need to do with each player.

To make it cleaner, you could use comprehensions:

user.receive_cards(5)    for user in users
user.place_first_cards() for user in users
user.receive_cards(3)    for user in users
...

This clearly illustrates each step of the game play, and doesn't have a repetitive line right after each one.

In fact, you could simplify this even further. If you keep a list of lambdas that show/do each of the different game moves (receive_cards, play_first_cards, etc.), then you easily loop through the list, calling each lambda.

Here's what the list might look like:

game_moves = [lambda user: user.receive_cards(5), lambda user: user.place_first_cards(), ...]

Then:

for game_move in game_moves:
    game_move(for user in users)

Note: I didn't have time to test this out, so this may not work. Leave a comment if there is an issue; I'll fix it later.

Now, with the list, each part of the game item by item is easy visible just a glance at the list.

If this, return true. Else that, return false.

I came across this in your code, inside your board_limit method:

if len(self.board[board_number - 1]) < 3:
    return True
else:
    return False

This is unnecessary. If the condition in the if statement evaluates to true, why don't you just return the conditional itself?

return len(self.board[board_number - 1]) < 3

Refactor methods

Look at your place_first_cards and place_cards methods. Then, put them in a diff tool. What's the different? Here:

while len(self.hand) != 0:

and:

self.hand.pop(0)

These are the only two lines that are different among these two methods. Everything else is exactly the same.

You should refactor this into one method that takes a parameter that tells if the cards are the first cards or not. Then, your new method would look like this:

def place_cards(self, is_first):
    print "Your time to play %s. Here is your hand. Pick a card and choose a row to place it" % self.name
    while len(self.hand) != (0 if is_first else 1): <--------
        print '-'.join(self.hand)
        for i in self.board:
            print i
        card_placed = int(raw_input("Choose a card (1 - %d): " % len(self.hand)))
        chosen_board = int(raw_input("Choose a board (1-3): "))
        if self.board_limit(chosen_board):
            self.board[chosen_board - 1].append(self.hand[card_placed - 1])
            self.hand.pop(card_placed - 1)
        else:
            print "The board is full. Choose another one."
    if not is_first:
        self.hand.pop(0) <------

This single method does exactly what those other two methods did, except for two small sections that can be "turned on or off" with a simple argument.

Now, your code would look like this for placing first cards:

user.place_cards(is_first=True)

Note: the is_first= is not necessary, but it increases readability, I think.

Now your code is simpler.

StackExchange Code Review Q#120418, answer score: 3