patternpythonMinor
How object oriented is my Crazy Eights game?
Viewed 0 times
eightsgamecrazyhoworientedobject
Problem
I have this Crazy Eights game (which at the moment does not include crazy cards):
```
class Card(object):
RANKS = ("2", "3", "4", "5", "6", "7", "8", "9", "T", "J", "Q", "K", "A")
SUITS = ("c", "d", "h", "s")
def __init__(self, rank, suit):
self.rank = rank
self.suit = suit
def __str__(self):
return self.rank + self.suit
def __eq__(self, other):
return self.rank == other.rank and self.suit == other.suit
def canPlayOn(self, other):
return self.rank == other.rank or self.suit == other.suit
class Hand(object):
def __init__(self):
self.cards = []
def __str__(self):
return " ".join([str(card) for card in self.cards])
def __bool__(self):
return bool(self.cards)
def give(self, other, card):
self.remove(card)
other.add(card)
def add(self, card):
self.cards.append(card)
def remove(self, card):
self.cards.remove(card)
class Pile(Hand):
@property
def topCard(self):
return self.cards[-1]
class DrawPile(Pile):
def populate(self):
for rank in Card.RANKS:
for suit in Card.SUITS:
self.add(Card(rank, suit))
def deal(self, others, cards = 7):
import random
random.shuffle(self.cards)
for _ in range(cards):
for hand in others:
self.give(hand, self.topCard)
class Player(Hand):
def playMove(self, discardPile, drawPile):
card = self.getCard(discardPile.topCard)
self.give(discardPile, card) if card else \
drawPile.give(self, drawPile.topCard)
def getCard(self, topCard):
card = input("This is your hand: " + str(self) +
". The is the top card: " + str(topCard) +
". Enter your card (enter PU to pick up a card): ")
while self.isInvalid(card, topCard):
card = input("Invalid input. Enter your card: ")
```
class Card(object):
RANKS = ("2", "3", "4", "5", "6", "7", "8", "9", "T", "J", "Q", "K", "A")
SUITS = ("c", "d", "h", "s")
def __init__(self, rank, suit):
self.rank = rank
self.suit = suit
def __str__(self):
return self.rank + self.suit
def __eq__(self, other):
return self.rank == other.rank and self.suit == other.suit
def canPlayOn(self, other):
return self.rank == other.rank or self.suit == other.suit
class Hand(object):
def __init__(self):
self.cards = []
def __str__(self):
return " ".join([str(card) for card in self.cards])
def __bool__(self):
return bool(self.cards)
def give(self, other, card):
self.remove(card)
other.add(card)
def add(self, card):
self.cards.append(card)
def remove(self, card):
self.cards.remove(card)
class Pile(Hand):
@property
def topCard(self):
return self.cards[-1]
class DrawPile(Pile):
def populate(self):
for rank in Card.RANKS:
for suit in Card.SUITS:
self.add(Card(rank, suit))
def deal(self, others, cards = 7):
import random
random.shuffle(self.cards)
for _ in range(cards):
for hand in others:
self.give(hand, self.topCard)
class Player(Hand):
def playMove(self, discardPile, drawPile):
card = self.getCard(discardPile.topCard)
self.give(discardPile, card) if card else \
drawPile.give(self, drawPile.topCard)
def getCard(self, topCard):
card = input("This is your hand: " + str(self) +
". The is the top card: " + str(topCard) +
". Enter your card (enter PU to pick up a card): ")
while self.isInvalid(card, topCard):
card = input("Invalid input. Enter your card: ")
Solution
I'm just going to look at your OOP design for this review, as you requested.
Your
I was initially thinking that your
In
I take issue with
Your
canPlayOn() method does not belong in the Card class. The purpose of your Card class should be to represent a card, and nothing more. You should be able to use the same Card class for Blackjack, Bridge, or any other card game. Logic specific to Crazy Eights does not belong there, and should probably be moved into the Player class.I was initially thinking that your
Hand should consist of a set of cards rather than an array, since the items are distinct and order doesn't matter. Then I saw that your Pile derives from Hand, and in Pile, order does matter. Maybe using an array is OK after all, if you accept that a Pile is a subclass of Hand. I think that such an inheritance relationship is more of a hack than an accurate model, but I can let it slide because it seems to work in this case.In
DrawPile.deal(), I would rename the others parameter to recipient_hands. Also imports should go at the beginning of the file.I take issue with
Player(Hand) and Computer(Hand). First of all, I would say that a player has a hand rather than a player is a hand. That suggest that composition should be favoured over inheritance. Also, the computer is a kind of player, so there should be an inheritance relationship there. Furthermore, there is a lot of redundant code between Player and Computer, differing mainly in what the output looks like. Perhaps your Player and Computer classes could be merged into a Crazy8Player class, with the constructor taking name, narrator, and strategy arguments.Context
StackExchange Code Review Q#32537, answer score: 8
Revisions (0)
No revisions yet.