5
\$\begingroup\$

this is my first code review post :) So I wrote this Black Jack game as part of an online Python course and would love some feedback about my coding style and whether or not I have correctly used a 'Pythonic' style in terms of the OOP.

Any general comments are also extremely welcome because I am making a move towards Python as serious commitment and want to follow the standard conventions as closely as possible.

Here is my code. Many thanks

Deck class:

import random
from card import Card


class Deck:
    '''
    A class for creating a Deck of Cards.
    A full deck of 52 cards is created and
    can be accessed as a list or as as a string.
    Deck exposes a <shuffle> method that will
    pseudo-randomly reorganise the list of cards
    '''

    __card_values = {
        'A': 1,
        '2': 2,
        '3': 3,
        '4': 4,
        '5': 5,
        '6': 6,
        '7': 7,
        '8': 8,
        '9': 9,
        '10': 10,
        'J': 10,
        'Q': 10,
        'K': 10,
    }

    __suits = ['Clubs', 'Spades', 'Hearts', 'Diamonds']

    def __init__(self):
        self.__deck = [Card(value + '-' + suit, self.__card_values[value])
                       for suit in self.__suits
                       for value in self.__card_values]

    @property
    def deck(self):
        return self.__deck

    @deck.setter
    def deck(self, val):
        self.__deck = val

    def shuffle(self):
        random.shuffle(self.__deck)

Card class:

class Card:
    '''
    A class for creating Card objects.
    Each Card object has a <face_value>
    which denotes the suit and its 
    value in the Deck. Each Card also
    has a <value> which denotes how many
    points the Card is worth in the game
    '''

    def __init__(self, face_value, value):
        self.__face_value = face_value
        self.__value = value

    @property
    def face_value(self):
        return self.__face_value

    @property
    def value(self):
        return self.__value

    @value.setter
    def value(self, val):
        self.__value = val

    def __str__(self):
        return f'{self.face_value}'

Hand class

class Hand:
    '''
    A class for creating a Hand of Cards.
    An instance of Hand takes a list of
    Card objects as an argument which 
    becomes the players hand for a single
    round. The <take_card> method adds a 
    new card to the hand
    '''

    def __init__(self, cards):
        self.__cards = cards

    def take_card(self, card):
        self.__cards.append(card)

    @property
    def cards(self):
        return self.__cards

    def __str__(self):
        card_string = ''
        for card in self.cards:
            card_string += card.face_value + '\n'
        return card_string

Actor class

class Actor:
    '''
    Super class for creating game
    participants, namely the Dealer and
    the human Player. An Actor must have
    a Hand and the human Actor must have
    some chips in order to play. <chips>
    is set to 0 by default so the Dealer
    will not have any. This class has 2
    abstract methods; <lose_bet> and
    <win_bet>. Each method effects the
    number of chips the Actor has. (For
    the Dealer, these methods will simply
    pass)
    '''

    def __init__(self, hand=None, chips=0, score=0):
        self.__hand = hand
        self.__chips = chips
        self.__score = score

    @property
    def hand(self):
        return self.__hand

    @hand.setter
    def hand(self, val):
        self.__hand = val

    @property
    def chips(self):
        return self.__chips

    @chips.setter
    def chips(self, val):
        self.__chips = val

    @property
    def score(self):
        return self.__score

    @score.setter
    def score(self, val):
        self.__score = val

    def lost_bet(self):
        raise NotImplementedError("Subclass must inherit this abstract method")

    def won_bet(self):
        raise NotImplementedError("Subclass must inherit this abstract method")


class Dealer(Actor):

    def lost_bet(self):
        pass

    def won_bet(self):
        pass


class Player(Actor):

    def lost_bet(self, val):
        self.chips -= val

    def won_bet(self, val):
        self.chips += val

black_jack.py

import random
import functools
from deck import Deck
from hand import Hand
from actor import Dealer, Player


def deal_cards(limit, max_cards):
    cards = [random.randint(0, limit) for _ in range(0, max_cards)]
    while True:
        if len(set(cards)) != len(cards):
            cards = [random.randint(0, limit) for _ in range(0, max_cards)]
        else:
            break
    return cards


def remove_cards_from_deck(deck, cards):
    cards.sort(reverse=True)
    for i in cards:
        deck.pop(i)
    return deck


def calc_score(player, cards):
    card_values = [card.value for card in cards]
    score = functools.reduce(lambda x, y: x+y, card_values)
    if score <= 11:
        for value in card_values:
            if value == 1 and (score + 10) <= 21:
                score += 10
    return score


def hit(deck, player):
    cards = deal_cards(len(deck)-1, 1)
    new_card = deck[cards[0]]
    player.hand.take_card(new_card)
    player.score = calc_score(player, player.hand.cards)
    deck = remove_cards_from_deck(deck, cards)
    return deck


def display_data(dealer, player, player_bet):
    print('\nDealer\'s hand:')
    print(dealer.hand.cards[0])
    print('\nPlayers hand:')
    print(player.hand)
    print(f'Current bet: {player_bet}')
    player.score = calc_score(player, player.hand.cards)
    print(f'Player Score: {player.score}')
    dealer.score = calc_score(dealer, dealer.hand.cards)


def has_won(dealer, player):
    print('\nDealers Hand:')
    print(dealer.hand)
    print(f"Dealer score: {dealer.score}")
    print(f"Player score: {player.score}")
    if player.score > 21:
        print('\nBust, you loose... boho')
        return False
    elif player.score <= 21 and player.score > dealer.score or (dealer.score > 21 and player.score <= 21):
        print('\nCongrats Player, you won!!')
        return True
    else:
        print('\nSorry Player, house wins... haha')
        return False


def start_game(dealer, player, player_bet):

    new_deck = Deck()
    new_deck.shuffle()

    cards = deal_cards(len(new_deck.deck)-1, 4)

    dealer_hand = Hand([new_deck.deck[cards[1]],
                        new_deck.deck[cards[3]]])
    player_hand = Hand([new_deck.deck[cards[0]],
                        new_deck.deck[cards[2]]])

    new_deck.deck = remove_cards_from_deck(new_deck.deck, cards)

    dealer.hand = dealer_hand
    player.hand = player_hand

    player.score = 0
    dealer.score = 0
    display_data(dealer, player, player_bet)

    while player.score <= 21:
        try:
            choice = input('\nHit or Stick? (H/S) ')
            if choice.lower() == 'h':
                new_deck.deck = hit(new_deck.deck, player)
                display_data(dealer, player, player_bet)
            if choice.lower() == 's':
                while dealer.score <= 17:
                    new_deck.deck = hit(new_deck.deck, dealer)
                break
            if dealer.score <= 17:
                new_deck.deck = hit(new_deck.deck, dealer)
        except Exception:
            print('What are you playing at?')

    if has_won(dealer, player):
        player.won_bet(player_bet)
    else:
        player.lost_bet(player_bet)


def run():
    print('\t\n==================== Welcome to the Black Jack Casino ====================\n')
    print('Rules:')
    print('''
      1. Aces are worth 1 or 11
      2. Numbered cards are worth their value
      3. Picture cards are worth 10
      4. The closest to 21 between you and the dealer is the winner
      5. If you go over 21 you loose
      6. You can only see one of the dealer's cards
      7. 'Hit' to take another card
      8. 'Stick' to keep what you have and compare with the dealer
      9. If you and the dealer have the same score which is less than or equal to 21,
         the house still wins
      10. Good luck!!\n
    ''')
    while True:
        try:
            player_chips = int(input('How many chips do you want to buy? '))
            break
        except Exception:
            print('Don\'t muck me about...\n')

    cash_out = False
    dealer = Dealer()
    player = Player()
    player.chips = player_chips

    aces = {'A-Clubs', 'A-Hearts', 'A-Diamonds', 'A-Spades'}

    while not cash_out and player.chips > 0:
        try:
            print(f'Chips: {player.chips}')
            player_bet = int(input('Place a bet: '))
            if player_bet > player.chips:
                print('You don\'t have the readies mate...\n')
            else:
                start_game(dealer, player, player_bet)
                try:
                    if (player.chips != 0):
                        quit = input('\nYou want to continue? (Y/N) ')
                        if quit.lower() == 'n':
                            cash_out = True
                    else:
                        break
                except Exception:
                    print('Don\'t talk rubbish...\n')
        except Exception:
            print('Don\'t waste my time...\n')
            continue

    try:
        play_again = input('\nFancy another game? (Y/N) ')
        if play_again.lower() == 'y':
            run()
        else:
            print('Next time then sucker...')
            return
    except Exception:
        print('Don\'t talk rubbish...')


run()
\$\endgroup\$
6
  • \$\begingroup\$ Are lost_bet and won_bet for Dealer supposed to be pass, or have you not implemented that yet? \$\endgroup\$
    – Linny
    Jan 21, 2020 at 23:13
  • \$\begingroup\$ They are supposed to pass since the Dealer has no chips to lose or win. I wanted the Dealer to inherit from Actor as well so it would get a 'hand' and a 'score', but there is no need tor it to use these methods. Does that make sense? \$\endgroup\$ Jan 21, 2020 at 23:29
  • \$\begingroup\$ Ah ok the whole class can just pass, I didn't know that. I have been studying a lot of Java where it is necessary to override methods in sub-classes, so I guess it's something I'll have to drop with Python. Thanks \$\endgroup\$ Jan 21, 2020 at 23:34
  • \$\begingroup\$ Please disregard my last comment, I didn't see the NotImplementedError. Coming from Java myself, it's understandable that you want subclasses to inherit all methods from a super class. This isn't always the case in Python. Since only one class actually uses these two methods, Player can have these methods in that class, instead of inheriting from Actor. \$\endgroup\$
    – Linny
    Jan 21, 2020 at 23:36
  • \$\begingroup\$ That makes sense. The Dealer class can just pass then still as it has no methods to implement? \$\endgroup\$ Jan 21, 2020 at 23:38

1 Answer 1

3
\$\begingroup\$

This answer is focused on one very specific part of this program.


Let's take a look at the Actor class. Specifically, these two methods:

def lost_bet(self):
    raise NotImplementedError("Subclass must inherit this abstract method")

def won_bet(self):
    raise NotImplementedError("Subclass must inherit this abstract method")

Now lets list how many subclasses actually use this method:

  • Player
  • Wait, just player?

Since Dealer is a subclass of Actor, I expected Dealer to utilize these methods as well. Not even Actor uses them in a meaningful way, except to make sure the subclasses write their own definition for them.

From our conversation in the comments, you state you came from Java. Now I can clearly see why you used inheritance in this way. This is unnecessary for Python. lost_bet and won_bet should be standalone methods in the Player class, since that subclass is the only class that uses these two methods in a meaningful way.

What I recommend is to keep Player exactly how it is. What I would change is the following:

  • Remove lost_bet and won_bet from the Actor class.
  • Set the Dealer class to pass.

For the Dealer point, something like this:

class Dealer(Actor):
    pass

Now you have a way to clear distinguish a Dealer and Actor from each other, without the two unnecessary methods.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thanks a lot that's really useful to know and makes inheritance a lot simpler. Much appreciated \$\endgroup\$ Jan 21, 2020 at 23:54

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.