Simulate a simplified version of Exploding Kittens in Python

Question

PURPOSE:

In order to get comfortable with Python classes and object-oriented programming, I've decided to simulate the Exploding Kittens card game. To make it easier to program, I've greatly simplified the rules of the game.

I'd really appreciate feedback on my code to identify errors in syntax, style, modeling, or expressiveness. Thanks!

RULES:

In this version, as with the original version, the goal of Exploding Kittens is to be the last player standing. (Original version rules here)

Each round, players take turns drawing from a deck filled with different types of cards. In this version of the game, there are only two types of cards: Regular and Exploding Kitten. If the player draws a Regular card, then they've survived that round. But when the player draws an Exploding Kitten card, they're dead! Players will want to avoid drawing an Exploding Kitten card for as long as they can.

Futhermore, in this version of the game, the player has only one possible action they can take each round. They MUST draw the top card from the deck. They cannot shuffle the deck, steal cards, etc. as is possible in the original version of the game. They can only draw the topmost card in the deck. Therefore, the outcome of the game is pure chance.

When only a single player remains, that player wins the game!

import random

class Card:
    def __init__(self, name):
        self.name = name

    def __str__(self):
        return self.name

    def __repr__(self):
        return str(self.name)

class Deck:
    def __init__(self, kittens = 4, regulars = 48):
        self.kittens = kittens
        self.regulars = regulars
        self.types = ['Exploding Kitten']*self.kittens + ['Regular']*self.regulars
        self.actual_cards = [Card(i) for i in self.types]

    def shuffle(self):
        random.shuffle(self.actual_cards)

    def get_top_card(self):
        card_to_be_returned = self.actual_cards.pop(0)
        return card_to_be_returned

    def get_four_cards(self): #for dealing the initial hand to each player
        four_cards = []
        for i in range(4):
            card_name_list = []
            for j in range(len(self.actual_cards)):
                card_name_list.append(self.actual_cards[j].name)
            regular_card_index = card_name_list.index('Regular')
            regular_card = self.actual_cards.pop(regular_card_index)
            four_cards.append(regular_card)
        return four_cards

class Player: 
    def __init__(self, ID, hand):
        self.player_ID = ID
        self.hand = hand
        self.alive = True

    def __str__(self):
        return self.player_ID

    def __repr__(self):
        return str(self.player_ID)

class ExplodingKittens():
    def __init__(self,num_players):
        self.num_players = num_players
        self.player_list = []

    def start_game(self): #set up game for first round of card draws
        self.deck_of_cards = Deck(self.num_players - 1, 53 - self.num_players)
        for player_ID in range(1, self.num_players + 1): #start w Player 1
            cards_for_player = self.deck_of_cards.get_four_cards() #Deal card to player
            player_ID = "Player " + str(player_ID)
            new_player = Player(player_ID, cards_for_player) #Save player ID and card
            self.player_list.append(new_player)
        self.deck_of_cards.shuffle()

    def play_round(self):
        for i in range(len(self.player_list)):
            top_card = self.deck_of_cards.get_top_card()
            print('{} drew {}'.format(self.player_list[i], top_card))
            if str(top_card) == 'Exploding Kitten':
                print('{} is dead!'.format(self.player_list[i]))
                self.player_list[i].alive = False
            alive = [self.player_list[j].alive for j in range(len(self.player_list))]
            if sum(alive) == 1:
                winner_index = alive.index(True)
                return '{} won the game!'.format(self.player_list[winner_index])

        player_list = [] #update player_list with only living players
        for i in range(len(self.player_list)): 
            if self.player_list[i].alive:
                player_list.append(self.player_list[i])
        self.player_list = player_list

    def decide_winner(self):
        while len(self.player_list) > 1:
            outcome = self.play_round()
            if outcome is not None:
                print(outcome)
                break

if __name__=="__main__":
    """run this code when executing module as a program 
    and not when importing its classes"""
    game = ExplodingKittens(5)
    game.start_game()
    print(game.player_list)
    game.decide_winner()

Answer 1

Looking at the construction of the deck_of_cards variable, you actually create the internal variable outside the __init__ function (inside the start_game function). So, that change would result in (the brackets are redundant too):

class ExplodingKittens:
    def __init__(self, num_players):
        self.num_players = num_players
        self.player_list = []
        self.deck_of_cards = None

Also, the constuction of the Deck, there are some defaults which we all know, 52 cards in a deck, so let's take advantage of that -

class Deck:
    def __init__(self, kittens=None):
        self.kittens = kittens or 4
        self.regulars = 52 - kittens
        ...

There's a good reason for not specifying a default value in the function defintion of the __init__, and that's because the value is only evaluated once at definition time. Specifying None as the argument then comparing verses a default value is safer (sorry if I haven't explained that well!).

This of course changes the construction of the deck_of_cards statement:

self.deck_of_cards = Deck(self.num_players - 1, 53 - self.num_players)

into:

 self.deck_of_cards = Deck(self.num_players - 1)

Which raises the questions regarding your design. You're using multiple controls to determine the winner, such as:

alive = [self.player_list[j].alive for j in range(len(self.player_list))]
if sum(alive) == 1:
    winner_index = alive.index(True)
    return '{} won the game!'.format(self.player_list[winner_index])

and also the:

while len(self.player_list) > 1:

in the decide_winner function (also you don't need that, when you create the game it should start playing immediately).

As a suggestion, I would update the design to have players in a list, and when a player obtains an Exploding Kitten, remove them from that list, and perform a len(players) used to determine if the game has finished (also the Player class no longer needs the self.alive parameter).

Having your game designed that way would also eliminate code like:

    player_list = []  # update player_list with only living players
    for i in range(len(self.player_list)):
        if self.player_list[i].alive:
            player_list.append(self.player_list[i])
    self.player_list = player_list

which would become redundant. Also, just a note on that type of code - you're checking for properties on objects via the index, rather than doing a statment like for player in players: which iterates through each object in the list (do you see the distinction?).

I hope this is enough to begin your rewrite, good luck!

Answer 2

• You can change Card and Player into collections.namedtuples.
• You don't need to define self.kittens, self.regulars or self.types on Deck.
• Change your types from raw strings to variables. Or even better an enum.
• Reduce the size of your variable names self.actual_cards can just be self.cards, self.deck_of_cards can just be self.deck.
• In get_top_card you don't need card_to_be_returned.
• Your function get_four_cards is pretty static. You may want to make this function more generic.
• You don't need self.num_players on ExplodingKittens.
• It'd be better if start_game doesn't mutate object state, make it return deck and players.

• In the future you may want to shuffle the deck before taking cards. This will mean people can then draw different cards.

  Note: you also want to keep your current shuffle, as get_four_cards will create bias.

• You may want to run start_game in ExplodingKittens.__init__.

• You can simplify play_round if you merge your two loops into one while loop. Do this by manually performing the same as range, but don't increment the index if you remove a player.
• I'd recommend you don't return data from play_round, just use it to exit the function.
• I recommend you make most of your class values 'privates', just prefix them with _. So deck becomes _deck.

Below is how I'd improve your code, I also made get_card and get_cards more generic. Possibly more than you'd need in your game.

import random
import collections
import enum

Card = collections.namedtuple('Card', 'type')
Player = collections.namedtuple('Player', 'id hand')


class CardTypes(enum.Enum):
    EXPLODING_KITTEN = 'Exploding Kitten'
    REGULAR = 'Regular'


class Deck:
    def __init__(self, kittens=4, regulars=48):
        self._cards = [
            Card(i) for i in (
                [CardTypes.EXPLODING_KITTEN] * kittens
                + [CardTypes.REGULAR] * regulars
            )
        ]

    def shuffle(self):
        random.shuffle(self._cards)

    def card_fn(self, key=None, exclude=None, include=None):
        null = object()

        def fn(value):
            if key:
                value = getattr(value, key, null)
            return (
                (exclude and value in exclude)
                or (include and value not in include)
            )
        return fn

    def get_card(self, fn=None):
        if fn is None:
            fn = lambda card: False

        card = None
        cards = []
        while card is None:
            card = self._cards.pop()
            if fn(card):
                cards.append(card)
                card = None
        self._cards.extend(reversed(cards))
        return card

    def get_cards(self, n, fn=None):
        return tuple(self.get_card(fn=fn) for _ in range(n))


class ExplodingKittens:
    def __init__(self, amount_players):
        self._deck, self._players = self.start_game(amount_players)

    def start_game(self, amount_players):
        deck = Deck(amount_players - 1, 53 - amount_players)
        deck.shuffle()
        exclude_kittens = deck.card_fn('type', exclude={CardTypes.EXPLODING_KITTEN})
        players = [
            Player(f'Player {id + 1}', list(deck.get_cards(4, exclude_kittens)))
            for id in range(amount_players)
        ]
        deck.shuffle()
        return deck, players

    def play_round(self):
        index = 0
        while index < len(self._players):
            player = self._players[index]
            card = self._deck.get_card()
            player.hand.append(card)
            print('{} drew {}'.format(player.id, card.type.value))

            if card.type is CardTypes.EXPLODING_KITTEN:
                print('{} is dead!'.format(player.id))
                self._players.pop(index)
            else:
                index += 1

            if len(self._players) == 1:
                print('{} won the game!'.format(self._players[0].id))
                return

    def decide_winner(self):
        while len(self._players) > 1:
            self.play_round()


if __name__ == "__main__":
    game = ExplodingKittens(5)
    print(game._players)
    game.decide_winner()

Answer 3

Functional approach

My suggestion is to use to use functional style as much as you can while you work with lists, because it will significantly reduce number lines of your code and will make it (code) more concise and readable. The first thing I would suggest to do is getting rid of for loops. Instead of them you can use list comprehension.

Modular code

In my experience is better to use five small functions or methods instead of one big. I always try to think in terms of atomic operations. One atomic operation is one function or method with single responsibility. To solve complex problem you just need to combine simple solutions in a correct way.

Remarks

1. class Card I suggest to use type of cards not names, because you have only two unique names. You can use enum to represent type of cards.
2. self.actual_cards = ... Word actual from property name actual_cards is not giving you any additional information, so you can leave only word cards
3. def get_top_card(self) In my opinion changing state (actual_cards) in method, which's prefix is get, is strange. I would suggest to use prefix pop or take. Body of method could be simplified to return self.actual_cards.pop(0)
4. class Deck I suggest to create some private methods to work with Deck. For example: _take_card(type): take first encountered card of given type; _take_cards(amount, type): take few cards of given type
5. def get_four_cards(self) Number of cards should not be hardcoded in method's name. It's better to have parameter for that. I suggest to rename this method to something like deal_regular_cards(number_of_cards).
6. class Player Parameter hand could be default, because logically you can have player without hand and for me (maybe only) is unclear that you need to pass hand as second parameter in constructor. I think it's better to have something like: Player(123, hand=[...]). Property player_ID could be simplified to id, because you are already in Player context
7. class ExplodingKittens In my opinion is better to initialize all properties in constructor: player_list (create all players in constructor) and deck_of_cards.
8. player_ID = "Player " + str(player_ID) You can move this logic to method __str__ of class Player
9. def decide_winner The name of this method is not match what it does. This is actually game loop. Choosing winner is in method play_round().