8
\$\begingroup\$

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()
asked Jun 15, 2018 at 2:25
\$\endgroup\$

3 Answers 3

6
\$\begingroup\$

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!

answered Jun 15, 2018 at 7:02
\$\endgroup\$
5
\$\begingroup\$
  • 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()
answered Jun 15, 2018 at 10:44
\$\endgroup\$
4
\$\begingroup\$

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().
answered Jun 15, 2018 at 9:02
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.