Python implementation of snap

I did an assessment task where I had to implement the game snap. The description:

Simulate a simplified game of snap between two computer players using N packs of cards (standard 52 card, 4 suit packs). A game of snap proceeds by shuffling N packs of cards and then drawing cards from the top of the shuffled pile. Each card drawn is compared to the previous card drawn and, if they match, all the cards drawn since the last match are randomly allocated to one of the players. The game continues until all the cards have been drawn from the shuffled pile. Any cards played without ending in a match once all the cards have been drawn are ignored. The winner is the player who has accumulated the most cards at the end of the game.

There are three possible matching conditions:

• The cards have the same suit.

• The cards have the same value.

• The cards have the same suit or the same value.

Your solution should ask:

• How many (N) packs of cards to use.

• Which of the three matching conditions to use.

Then simulate a game being played and display the winner.

My solution:

task/main.py:

from utils import read_options_from_stdin

from snap.game import Game

if __name__ == '__main__':
    number_of_packs, matching_condition = read_options_from_stdin()
    game = Game.init_game(number_of_packs, matching_condition)

    while not game.finished:
        game.turn()

    print game.winner

task/utils.py:

```
from snap.consts import MATCHING_CONDITIONS

def read_options_from_stdin():
number_of_packs = None
while type(number_of_packs) != int:
try:
number_of_packs = int(raw_input('Please tell me how many packs to use: ').strip())
except ValueError:
print 'Please provide a number'

matching_condition = None
while matching_condition not in MATCHING_CONDITIONS:
if matching_condition is not None:
print 'wrong matching condition.'

matching_condition = raw_input('''
Please choose a

Fragmentation

Each file is rather short. I assume that you probably wanted to have a very dedicated scope for each file, but splitting the logic that much makes it a bit harder to follow through, especially on small projects like that. I would have used 2 files, one dedicated to decks and cards, and the other one for the game itself, with player and input handling.

OOP

You define __init__ methods on subclasses of cards that just use super on generic arguments. There is absolutely no need to that as it is the default behaviour in Python. What is left is just 3 classes with the __eq__ method which thus would be better of as functions:

def compare_cards_by_value(card1, card2):
    return card1.value == card2.value

def comapare_cards_by_suit(card1, card2):
    return card1.suit == card2.suit

def compare_cards_by_suit_or_value(card1, card2):
    return compare_cards_by_suit(card1, card2) or compare_cards_by_value(card1, card2)

Which means Cards would only be a class to store attributes, but a namedtuple would fit better there, as it would make a Card immutable:

Card = namedtuple('Card', 'value suit')

Your use of classmethod is also unconventional. Especially in the Player class. If you want to be able to build a Player without specifying the amount of cards, it would be better to give this parameter a default value in the constructor:

class Player(object):
    def __init__(self, name, number_of_cards=0):
        self.cards = number_of_cards
        self.name = name

    def add_pile(self, number_of_cards):
        self.cards += number_of_cards

    def __str__(self):
        return '{p.name} {p.cards}'.format(p=self)

Pretty much the same argument for Deck:

class Deck:
    def __init__(self, cards=None):
        if cards is None:
            cards = [Card(value, suit) for value in VALUES for suit in SUITS]
        self.cards = cards

get_empty_deck being only Deck([]) doesn't add much value here. What could have been a better classmethod, however, is a merge method:

@classmethod
    def merge(cls, number_of_decks):
        return sum((cls() for _ in range(number_of_decks)), cls([]))

but this writing requires you to write an __add__ method instead of the __iadd__ one:

def __add__(self, other):
        return self.__class__(self.cards + other.cards)

Moreover some methods of this class would be better of as special ("magic") methods:

def __len__(self):
        return len(self.cards)

    def __nonzero__(self):
        return bool(self.cards)

Which will let you use more pythonic construct such as while my_deck…

I also feel that there is not much added value to write a Game class and a function or two would suffice, example code latter.

Handling user input

I don't know if I misunderstood slightly the requirements, but I thought user input should be handled from the command line. Which is ofter neater as you can use modules that encapsulate boilerplate code from sanitizing it. For example, using argparse:

def read_options():
    parser = argparse.ArgumentParser(description="A snap game simulator. "
            "Perform comparisons on suits and values of consecutively drawn "
            "cards to know whether or not give previously drawn cards to a player")
    parser.add_argument('number_of_decks', type=int)
    group = parser.add_mutually_exclusive_group()
    group.add_argument('--no-suits', action='store_true', help='Disable suits comparison')
    group.add_argument('--no-values', action='store_true', help='Disable values comparison')

    args = parser.parse_args()
    if args.no_suits:
        compare_cards = compare_cards_by_value
    elif args.no_values:
        compare_cards = compare_cards_by_suits
    else:
        compare_cards = compare_cards_by_suits_or_value

    return args.number_of_decks, compare_cards

Proposed improvements

cards.py

```
import random
from collections import namedtuple

class Card(namedtuple('Card', 'value suits')):
SUITS = ('spades', 'hearts', 'diamonds', 'clubs')
VALUES = (
# No '1' because of 'ACE'
'2', '3', '4', '5', '6', '7', '8',
'9', '10', 'J', 'Q', 'K', 'ACE'
)

def __str__(self):
return '{c.value} of {c.suit}'.format(c=self)

def compare_cards_by_value(card1, card2):
return card1.value == card2.value

def comapare_cards_by_suit(card1, card2):
return card1.suit == card2.suit

def compare_cards_by_suit_or_value(card1, card2):
return compare_cards_by_suit(card1, card2) or compare_cards_by_value(card1, card2)

class Deck(object):
def __init__(self, cards=None):
if cards is None:
cards = [Card(value, suit)
for value in Card.VALUES
for suit in Card.SUITS]
self.cards = cards

def __add__(self, other):
return self.__class__(self.cards + other.cards)

@classmethod
def merge(cls, number_of_decks):
retur

StackExchange Code Review Q#134853, answer score: 4