patternpythonModerate
Simple card game to learn OOP
Viewed 0 times
simplecardgamelearnoop
Problem
My goal was to get my hands dirty in OOP by designing and using classes and getting started with inheritance and other OOP concepts.
I have written a very small code to play a card Game called "War". The rules are simple. Each person playing the game is given 1 card. The person with the highest card wins. I am not worrying too much about Ties right now. My code does work but I wanted feedback on my OOP usage.
```
import itertools
import random
class Cards:
def __init__(self):
self.suits = ['Spades','Hearts','Diamonds','Clubs']
self.values = range(1,14)
self.ActualCards = [] #Empty List to Append
for Card in itertools.product(self.suits,self.values):
self.ActualCards.append(Card) #Cartesian Product to Create Deck
def GetRandomCard(self):
RandomNumber = random.randint(0,51)
CardToBeReturned = self.ActualCards[RandomNumber] #Generates Random Card
return(CardToBeReturned)
class Player:
def __init__(self,ID,Card):
self.PlayerID = ID
self.CardForPlayer = Card
class Game:
def __init__(self,NameOfGame):
self.name = NameOfGame
class SimpleWar(Game):
def __init__(self,NumberOfPlayers):
self.NumberOfPlayers = NumberOfPlayers
self.PlayerList = []
def StartGame(self):
DeckOfCards = Cards()
for playerID in range(0,self.NumberOfPlayers):
CardForPlayer = DeckOfCards.GetRandomCard() #Deal Card to Player
NewPlayer = Player(playerID,CardForPlayer) #Save Player ID and Card
self.PlayerList.append(NewPlayer)
self.DecideWinner()
def DecideWinner(self):
WinningID = self.PlayerList[0] #Choose Player 0 as Potential Winner
for playerID in self.PlayerList:
if(playerID.CardForPlayer[1]>WinningID.CardForPlayer[1]):
WinningID = playerID #Find the Player with Highest Card
print "Winner is Player "+str(WinningID.PlayerID)
print "Her Card was
I have written a very small code to play a card Game called "War". The rules are simple. Each person playing the game is given 1 card. The person with the highest card wins. I am not worrying too much about Ties right now. My code does work but I wanted feedback on my OOP usage.
```
import itertools
import random
class Cards:
def __init__(self):
self.suits = ['Spades','Hearts','Diamonds','Clubs']
self.values = range(1,14)
self.ActualCards = [] #Empty List to Append
for Card in itertools.product(self.suits,self.values):
self.ActualCards.append(Card) #Cartesian Product to Create Deck
def GetRandomCard(self):
RandomNumber = random.randint(0,51)
CardToBeReturned = self.ActualCards[RandomNumber] #Generates Random Card
return(CardToBeReturned)
class Player:
def __init__(self,ID,Card):
self.PlayerID = ID
self.CardForPlayer = Card
class Game:
def __init__(self,NameOfGame):
self.name = NameOfGame
class SimpleWar(Game):
def __init__(self,NumberOfPlayers):
self.NumberOfPlayers = NumberOfPlayers
self.PlayerList = []
def StartGame(self):
DeckOfCards = Cards()
for playerID in range(0,self.NumberOfPlayers):
CardForPlayer = DeckOfCards.GetRandomCard() #Deal Card to Player
NewPlayer = Player(playerID,CardForPlayer) #Save Player ID and Card
self.PlayerList.append(NewPlayer)
self.DecideWinner()
def DecideWinner(self):
WinningID = self.PlayerList[0] #Choose Player 0 as Potential Winner
for playerID in self.PlayerList:
if(playerID.CardForPlayer[1]>WinningID.CardForPlayer[1]):
WinningID = playerID #Find the Player with Highest Card
print "Winner is Player "+str(WinningID.PlayerID)
print "Her Card was
Solution
Syntax Error
In
Style Conventions
Use
For readability, please add one space after each comma, and one newline between functions definitions.
Modeling
Your
In a real-life card game, you would deal a card from the deck to each player, without replacement. In your code, you deal with replacement (i.e., there is a chance of dealing the same card to both players). A more realistic simulation would do a
Expressiveness
In Python, you can usually avoid creating an empty list and appending to it:
Instead, you should be able to build it all at once:
In
Cards.GetRandomCard(), the return statement is incorrectly indented. (Also, I would recommend writing the return without parentheses.)Style Conventions
Use
lower_case_names for variables and methods.For readability, please add one space after each comma, and one newline between functions definitions.
Modeling
Your
Game class isn't useful. You never set or use NameOfGame. I recommend getting rid of the Game class altogether.In a real-life card game, you would deal a card from the deck to each player, without replacement. In your code, you deal with replacement (i.e., there is a chance of dealing the same card to both players). A more realistic simulation would do a
random.shuffle() on the array. When dealing, you would pop() a card from the list to remove it from the deck. 51 is a "magic" number; you should use len(self.ActualCards) - 1.Cards is really a Deck. Rather than just a tuple of strings, you should have a Card class representing a single card. The Card class should have a __str__() method that returns a string such as "Ace of diamonds". If you also define comparison operators, then you can determine the winner using max(self.PlayerList, key=lambda player: player.CardForPlayer).Expressiveness
In Python, you can usually avoid creating an empty list and appending to it:
self.ActualCards = [] #Empty List to Append
for Card in itertools.product(self.suits,self.values):
self.ActualCards.append(Card) #Cartesian Product to Create DeckInstead, you should be able to build it all at once:
self.actual_cards = list(itertools.product(self.suits, self.values))Code Snippets
self.ActualCards = [] #Empty List to Append
for Card in itertools.product(self.suits,self.values):
self.ActualCards.append(Card) #Cartesian Product to Create Deckself.actual_cards = list(itertools.product(self.suits, self.values))Context
StackExchange Code Review Q#42107, answer score: 18
Revisions (0)
No revisions yet.