patternpythonMinor
Abstract card game code in Python3
Viewed 0 times
abstractcardgamepython3code
Problem
I've written up some generic code for a card-game and would like to hear any and all suggestions for how to improve this code further, in any way shape or form.
The code is on github or pasted directly below:
Card class:
CardFactory class
Deck class
```
from random import *
class AbstractDeck:
"""Abstrac
The code is on github or pasted directly below:
Card class:
class AbstractCard:
"""Abstract class for handling comparisons between cards based on rank"""
def __init__(self, value=None, suit=None, rank=None):
self.value = value
self.suit = suit
self.rank = rank
def __lt__(self, other):
if isinstance(other, AbstractCard):
return self.rank other.rank
raise TypeError("Cannot compare to non-card types")
def __ge__(self, other):
if isinstance(other, AbstractCard):
return self.rank >= other.rank
raise TypeError("Cannot compare to non-card types")
def __repr__(self):
data = (self.value, self.suit, self.rank)
return "(value:{}, suit:{}, rank:{})".format(*data)
def __str__(self):
data = (self.value, self.suit)
return "{} of {}".format(*data)CardFactory class
from AbstractCard import AbstractCard
class AbstractCardFactory:
"""Factory for making AbstractCards"""
def __init__(self, values=None, suits=None, rank_function=None):
self.values = values
self.suits = suits
self.rank_function = rank_function
def __repr__(self):
return "Values: {}\nSuits: {}".format(self.values, self.suits)
def __str__(self):
return self.__repr__()
def generate(self):
if self.rank_function:
for suit in self.suits:
for value in self.values:
card = AbstractCard(value, suit)
card.rank = rank_function(card)
yield card
else:
for suit in self.suits:
for value in self.values:
yield AbstractCard(value, suit)Deck class
```
from random import *
class AbstractDeck:
"""Abstrac
Solution
In your
This is unwise - it pollutes the namespace with many names you aren't using, and makes it harder to tell where the one you are using came from. In this case, you only want one name from
is an improvement. Better still, you can factor out the awkward subtraction:
using
In case you weren't aware of it, there is also a
You have a potential error in
This doesn't return a copy; if the calling function mutates the hand, it will affect the player too. To return a copy, do e.g.
In general (and I apologise if this sounds harsh), much of your code seems either so abstract as to be pointless (
I have suggested a class structure for Blackjack elsewhere; you could adapt this to your needs. Note e.g. the separation of a
This is by no means a perfect structure - looking at it now, for instance, perhaps
If you do keep the card factory, note you can simplify:
AbstractDeck class, you:from random import *This is unwise - it pollutes the namespace with many names you aren't using, and makes it harder to tell where the one you are using came from. In this case, you only want one name from
random, sofrom random import randintis an improvement. Better still, you can factor out the awkward subtraction:
j = randint(i, num_cards - 1)using
randrange (which is half-open, like range) instead:j = randrange(i, num_cards)In case you weren't aware of it, there is also a
random.shuffle, which apparently uses Fisher-Yates and saves you from implementing it yourself.You have a potential error in
AbstractPlayer:def hand(self):
"""Returns a copy of the player's hand"""
return self.cardsThis doesn't return a copy; if the calling function mutates the hand, it will affect the player too. To return a copy, do e.g.
return self.cards[:]. Alternatively, alter the docstring.In general (and I apologise if this sounds harsh), much of your code seems either so abstract as to be pointless (
AbstractGame) or insufficiently abstract as to be useful (AbstractCard - what about games where suits are important, or games like Uno where some cards are unordered? There are more exceptions than rules here!) I think the card factory is unnecessary - does this logic not belong in a Deck?I have suggested a class structure for Blackjack elsewhere; you could adapt this to your needs. Note e.g. the separation of a
Player (where the I/O and/or playing logic sit, along with e.g. the name and current balance for betting games), and a Hand (where the player's cards sit). This is by no means a perfect structure - looking at it now, for instance, perhaps
Deck and Hand could have some common abstract ancestor CardCollection. Also, you will find that different games have e.g. PokerHand subclasses for whatever specific needs you have - the value of a specific set of cards will depend on the game being played.If you do keep the card factory, note you can simplify:
from itertools import product
...
def generate(self):
for suit, value in product(self.suits, self.values):
card = AbstractCard(value, suit)
if self.rank_function is not None:
card.rank = rank_function(card)
yield cardCode Snippets
from random import *from random import randintj = randint(i, num_cards - 1)j = randrange(i, num_cards)def hand(self):
"""Returns a copy of the player's hand"""
return self.cardsContext
StackExchange Code Review Q#58925, answer score: 6
Revisions (0)
No revisions yet.