Card game using OOP

Question

I already know that I shouldn't really use the global function and that how I judge who wins is also really inefficient so I would appreciate improvements for that. I watched a tutorial about Object Oriented Programming and this was my first attempt at using what i learned. I redid a game I did without OOP.

The Rules for the Game are :

• Player 1 takes the top card from the deck.

• Player 2 takes the next card from the deck. If both players have a card of the same colour, the player with the highest number wins.

• If both players have cards with different colours, the winning colour is shown in the table.

Card	Card	Winner
Red	Black	Red
Yellow	Red	Yellow
Black	Yellow	Black

• The winner of each round keeps both cards.
• The players keep playing until there are no cards left in the deck
• Players need to be validated

from random import shuffle 
Ranks = [ "1", "2", "3", "4", "5", "6", "7","8", "9", "10"]
Colours = [ "Y", "R", "B"]
class Card() :
 def __init__(self,rank,colour):
 self.rank = rank
 self.colour = colour
 def get_card(self) :
 return self.rank + self.colour
 def get_move(self) :
 return self.rank
 def get_colour(self) :
 return self.colour
class Deck() :
 def __init__(self) :
 self.deck = []
 self.tempdeck = []
 for Colour in Colours :
 for Rank in Ranks :
 self.tempdeck.append(str(Rank) + "-" + str(Colour))
 for x in self.tempdeck :
 s = x.split("-")
 x = Card(s[0],s[1])
 self.deck.append(x)
 self.shuffle_deck()
 
 def len(self) :
 return len(self.deck)
 
 def add_card(self,card) :
 self.deck.append(card)
 
 def remove_card(self,card) :
 self.deck.remove(card)
 def shuffle_deck(self) :
 shuffle(self.deck)
class Hand(Deck):
 def __init__(self,label) :
 self.deck = []
 self.label = label
 self.Score = 0
 def PrintCard(self) :
 Cards = []
 for x in self.deck :
 Cards.append(x.get_card())
 return str(Cards)
def WinnerDecider() :
 if P1Hand.len() > P2Hand.len() :
 print("Player 1 wins with, " + str(P1Hand.len() - P2Hand.len()) + "More Cards than Player 2" )
 print("Your Cards were:" , P1Hand.PrintCard())
 elif P1Hand.len() < P2Hand.len() :
 print("Player 2 wins with, " + str(P2Hand.len() - P1Hand.len()) + " More Cards than Player 1" )
 print("Your Cards were:" , P2Hand.PrintCard())
 elif P1Hand.len() == P2Hand.len() :
 print("Draw")
def RoundJudge() :
 if P1Move.get_colour() == "R" :
 if P2Move.get_colour() == "B" :
 P1Hand.add_card(P1Move)
 P1Hand.add_card(P2Move)
 print("P1 Won Got these cards:" + P1Move.get_card() + "," + P2Move.get_card())
 else :
 P2Hand.add_card(P1Move)
 P2Hand.add_card(P2Move)
 print("P2 Won Got these cards:" + P1Move.get_card() + "," + P2Move.get_card())
 if P1Move.get_colour() == "Y" :
 if P2Move.get_colour() == "R" :
 P1Hand.add_card(P1Move)
 P1Hand.add_card(P2Move)
 print("P1 Won Got these cards:" + P1Move.get_card() + "," + P2Move.get_card())
 else :
 P2Hand.add_card(P1Move)
 P2Hand.add_card(P2Move)
 print("P2 Won Got these cards:" + P1Move.get_card() + "," + P2Move.get_card())
 if P1Move.get_colour() == "B" :
 if P2Move.get_colour() == "Y" :
 P1Hand.add_card(P1Move)
 P1Hand.add_card(P2Move)
 print("P1 Won Got these cards:" + P1Move.get_card() + "," + P2Move.get_card())
 else :
 P2Hand.add_card(P1Move)
 P2Hand.add_card(P2Move)
 print("P2 Won Got these cards:" + P1Move.get_card() + "," + P2Move.get_card())
def Game() : 
 global P1Move
 global P2Move
 while Deck.len() > 0 :
 P1Choice = input(P1Name + " Press Enter to take your turn: ")
 if P1Choice == "" :
 P1Move = Deck.deck[0]
 Deck.remove_card(P1Move)
 print("You got: " , P1Move.get_card())
 P2Choice = input(P2Name + " Press Enter to take your turn: ")
 if P2Choice == "" :
 P2Move = Deck.deck[0]
 Deck.remove_card(P2Move)
 print("You got: " , P2Move.get_card())
 RoundJudge()
 WinnerDecider()
 
def Start() :
 global P1Hand
 global P2Hand
 global Deck
 global P1Name
 global P2Name
 P1Name = input("Name: ")
 P2Name = input("Name: ")
 Deck = Deck()
 P1Hand = Hand(P1Name)
 P2Hand = Hand(P2Name)
 Game()
def Login() :
 User1 = input("Enter Username:")
 Pass1 = input("Enter Password:")
 User2 = input("Enter Username:")
 Pass2 = input("Enter Password:")
 if User1 == "" and Pass1 == "" and User2 == "" and Pass2 == "" :
 Start()
 else :
 Login()
Login()

• Cris Luengo: Your game is biased! Player 1 always wins if both players draw the same color.
• Reinderien: @CrisLuengo Ish. It's biased toward player 2, not player 1.
• Cris Luengo: @Reinderien You're right. Good catch.

Answer 1

Most Python programmers follow PEP 8 (the official Python style guide) when writing code. I am probably not as concerned as most in following the exact style, but some of those guidelines do improve legibility. For example, adding empty lines around function and class definitions makes it easier to see the code structure. But the most important style guideline, which is true for any language you program in, is consistency within a code base. If you put a space before the : at the end of a def statement, do that for all such statements. Your class Hand(Deck): line breaks the consistency.

This is Deck.__init__():

 self.deck = []
 self.tempdeck = []
 for Colour in Colours :
 for Rank in Ranks :
 self.tempdeck.append(str(Rank) + "-" + str(Colour))
 for x in self.tempdeck :
 s = x.split("-")
 x = Card(s[0],s[1])
 self.deck.append(x)
 self.shuffle_deck()

tempdeck is not used outside this function, then why make it a member of the class? Don't do self.tempdeck, just do tempdeck. Then it will not stick around after the function finishes.

But tempdeck is not needed at all. It just adds confusion and work. Why not just do

 self.deck = []
 self.tempdeck = []
 for Colour in Colours :
 for Rank in Ranks :
 self.deck.append(Card(Rank, Colour))
 self.shuffle_deck()

RoundJudge() has a bug: it does not correctly implement the case where both players get a card with the same color. Instead of letting the higher number win, player 2 always wins.

This function also has a lot of redundancy. The lines

 P1Hand.add_card(P1Move)
 P1Hand.add_card(P2Move)
 print("P1 Won Got these cards:" + P1Move.get_card() + "," + P2Move.get_card())
 else :
 P2Hand.add_card(P1Move)
 P2Hand.add_card(P2Move)
 print("P2 Won Got these cards:" + P1Move.get_card() + "," + P2Move.get_card())

are repeated three times. Instead, first find out who wins, then put the cards in the hand of the winner. For example,

def RoundJudge() :
 if P1Move.get_colour() == "R" :
 if P2Move.get_colour() == "B" :
 winner = P1Hand
 else:
 winner = P2Hand
 if P1Move.get_colour() == "Y" :
 if P2Move.get_colour() == "R" :
 winner = P1Hand
 else :
 winner = P2Hand
 if P1Move.get_colour() == "B" :
 if P2Move.get_colour() == "Y" :
 winner = P1Hand
 else :
 winner = P2Hand
winner.add_card(P1Move)
winner.add_card(P2Move)

(the comparison of the colors could be done in a prettier way as well, but let's keep it simple for now.)

You are using global variables there, as well as in WinnerDecider(). Why not make a class Game that contains the deck and the two player's hands, and make RoundJudge() and WinnerDecider() class methods? I mean, if you go for an OOP design, you might as well go all the way. :)

Note that Hand does not need to inherit from Deck, since it doesn't need to store cards at all. The only important information is how many cards are in the hand. All you need is a counter.

Taking a card from the deck is also awkward:

 P1Move = Deck.deck[0]
 Deck.remove_card(P1Move)

I would prefer something like

 P1Move = Deck.take()

The take() method here is the classical POP operation, it extracts the top element from the deck. Note that your remove_card (which uses list.remove()) needs to do a lookup to find the element P1Move, then then removes it. But you are always removing the top element, so the lookup is redundant. list.pop() is the right operation to uses. Without arguments, pop() removes and returns the last element in the list. You could do pop(0), or you could just do the natural thing and take cards from the end of the list. It does not matter which way you read the list anyway.

• Flavour: Thank you, I really wished that I used what you said about a game class would've make things easier. I stored the cards so that I could print them out the end, it was probably unnecessary though.Thanks again!

Answer 2

The formatting is pretty rough; follow PEP8 and get an IDE that points out formatting suggestions to you.

Ranks should be RANKS (due to being a global constant) and should only be a call to range(); its elements should be integers and not strings. But also, your game logic ignores the rank entirely. You should probably pay attention to it when the dealt colours match.

Colours should be an Enum.

get_move and get_colour should be deleted. get_card should be re-framed as an override of __str__. Consider representing Card as a NamedTuple for immutability and simple construction.

Hand ignores the card initialisation code in Deck, which suggests that there should be a third abstract parent of the two classes that doesn't take an opinion on card initialisation.

PrintCard doesn't print, so just call it __str__.

Your function names need to be action phrases; so decide_winner instead of WinnerDecider.

Simplify your win decision logic by comparing colour values; one way to do this is normalise the values so that the distance between them is always 1.

Don't use globals.

You don't use the result from Press enter to take your turn, so don't save it to a variable.

Never input() to get a password; use getpass() instead.

Don't print P1/P2; use the player labels that you collected earlier.

Add a __main__ guard.

If login fails, don't recurse; just loop.

Suggested

from enum import Enum
from getpass import getpass
from random import shuffle
from time import sleep
from typing import NamedTuple
class Colour(Enum):
 # Red > Black, Yellow > Red, Black > Yellow
 YELLOW = 0
 BLACK = 1
 RED = 2
RANKS = range(1, 11)
class Card(NamedTuple):
 rank: int
 colour: Colour
 def __str__(self) -> str:
 return f'{self.rank}{self.colour.name[0]}'
class Pile:
 cards: list[Card]
 def __init__(self) -> None:
 raise NotImplementedError()
 def len(self) -> int:
 return len(self.cards)
 def add_card(self, card: Card) -> None:
 self.cards.append(card)
 def remove_card(self, card: Card) -> None:
 self.cards.remove(card)
 def shuffle(self) -> None:
 shuffle(self.cards)
 def deal(self) -> Card:
 return self.cards.pop(0)
 def __str__(self) -> str:
 return ', '.join(str(card) for card in self.cards)
class Deck(Pile):
 def __init__(self) -> None:
 self.cards = [
 Card(rank, colour)
 for colour in Colour
 for rank in RANKS
 ]
 self.shuffle()
class Hand(Pile):
 def __init__(self, name: str) -> None:
 self.cards = []
 self.label = name
def decide_winner(p1_hand: Hand, p2_hand: Hand) -> None:
 if p1_hand.len() == p2_hand.len():
 print('Draw')
 return
 if p1_hand.len() > p2_hand.len():
 winner, loser = p1_hand, p2_hand
 else:
 loser, winner = p1_hand, p2_hand
 diff = winner.len() - loser.len()
 print(f'{winner.label} wins with {diff} more cards than {loser.label}')
 print(f"{winner.label}'s cards were: {winner}")
def judge_round(
 p1_move: Card, p2_move: Card,
 p1_hand: Hand, p2_hand: Hand,
) -> None:
 c1, c2 = p1_move.colour.value, p2_move.colour.value
 if c1 == c2:
 # winner = p2_hand # p1 == p2: p2 wins
 # alternative tie-breaker: compare rank
 winning_rank, winner = max(
 (p1_move.rank, p1_hand),
 (p2_move.rank, p2_hand),
 )
 else:
 # Red > Black, Yellow > Red, Black > Yellow
 if max(c1, c2) == 2:
 if c1 == 0: c1 = 3
 elif c2 == 0: c2 = 3
 # At this point, the distance between c1 and c2 will always be 1, and
 # the higher value will be the winner
 winning_colour, winner = max(
 (c1, p1_hand),
 (c2, p2_hand),
 )
 winner.add_card(p1_move)
 winner.add_card(p2_move)
 print(f'{winner.label} won, got these cards: {p1_move}, {p2_move}\n')
def run_game(deck: Deck, p1_hand: Hand, p2_hand: Hand) -> None:
 while deck.len() > 0:
 moves = []
 for hand in (p1_hand, p2_hand):
 input(f'{hand.label}, press enter to take your turn.')
 move = deck.deal()
 print(f'You got: {move}')
 moves.append(move)
 judge_round(*moves, p1_hand, p2_hand)
 decide_winner(p1_hand, p2_hand)
def login() -> None:
 for player in range(1, 3):
 while True:
 username = input(f'Enter username for player {player}: ')
 password = getpass(f'Enter password for player {player}: ')
 if username == '' and password == '':
 break
 print('Invalid credentials')
 sleep(1)
 print('Logged in')
def main() -> None:
 login()
 p1_name = input('Player 1 Name: ')
 p2_name = input('Player 2 Name: ')
 print()
 run_game(
 deck=Deck(),
 p1_hand=Hand(p1_name),
 p2_hand=Hand(p2_name),
 )
if __name__ == '__main__':
 main()

Output

Enter username for player 1: 
Enter password for player 1: 
Logged in
Enter username for player 2: 
Enter password for player 2: 
Logged in
Player 1 Name: Julio
Player 2 Name: Fred
Julio, press enter to take your turn.
You got: 8B
Fred, press enter to take your turn.
You got: 5R
Fred won, got these cards: 8B, 5R
Julio, press enter to take your turn.
You got: 4R
Fred, press enter to take your turn.
You got: 4Y
Fred won, got these cards: 4R, 4Y
Julio, press enter to take your turn.
You got: 3B
Fred, press enter to take your turn.
You got: 7B
Fred won, got these cards: 3B, 7B
Julio, press enter to take your turn.
You got: 1R
Fred, press enter to take your turn.
You got: 3Y
Fred won, got these cards: 1R, 3Y
Julio, press enter to take your turn.
You got: 6R
Fred, press enter to take your turn.
You got: 8R
Fred won, got these cards: 6R, 8R
Julio, press enter to take your turn.
You got: 8Y
Fred, press enter to take your turn.
You got: 2B
Fred won, got these cards: 8Y, 2B
Julio, press enter to take your turn.
You got: 10B
Fred, press enter to take your turn.
You got: 9B
Julio won, got these cards: 10B, 9B
Julio, press enter to take your turn.
You got: 7R
Fred, press enter to take your turn.
You got: 7Y
Fred won, got these cards: 7R, 7Y
Julio, press enter to take your turn.
You got: 10R
Fred, press enter to take your turn.
You got: 2R
Julio won, got these cards: 10R, 2R
Julio, press enter to take your turn.
You got: 2Y
Fred, press enter to take your turn.
You got: 3R
Julio won, got these cards: 2Y, 3R
Julio, press enter to take your turn.
You got: 1Y
Fred, press enter to take your turn.
You got: 5B
Fred won, got these cards: 1Y, 5B
Julio, press enter to take your turn.
You got: 9R
Fred, press enter to take your turn.
You got: 4B
Julio won, got these cards: 9R, 4B
Julio, press enter to take your turn.
You got: 10Y
Fred, press enter to take your turn.
You got: 5Y
Julio won, got these cards: 10Y, 5Y
Julio, press enter to take your turn.
You got: 1B
Fred, press enter to take your turn.
You got: 6B
Fred won, got these cards: 1B, 6B
Julio, press enter to take your turn.
You got: 6Y
Fred, press enter to take your turn.
You got: 9Y
Fred won, got these cards: 6Y, 9Y
Fred wins with 10 more cards than Julio
Fred's cards were: 8B, 5R, 4R, 4Y, 3B, 7B, 1R, 3Y, 6R, 8R, 8Y, 2B, 7R, 7Y, 1Y, 5B, 1B, 6B, 6Y, 9Y

• Flavour: Thank you I don't really know all the coding conventions for like names of class that's why my name's are all over the place and I didn't even know getpass() existed. Can I ask what you mean by making colours an enum and what you meant by a adding main guard
• Reinderien: class Colour(Enum):; see docs.python.org/3/library/enum.html
• Reinderien: realpython.com/python-main-function