10
\$\begingroup\$

I made a simple text based Blackjack game in Python 3 to help me implement OOP concepts. I only made one function outside of my classes which takes the user's name at the beginning and then establishes a table with the player/dealer.

I'm mostly interested in improving my OOP, so I would appreciate your feedback. I could have done more with side-best, splitting hands, etc... but I felt this was enough for this project to see how my OOP implementation was going.

from random import shuffle


# I'm creating a table class where the rest of the object will reside to play the game
# this will allow different object to interact with each other 'on the table'
class Table(object):

    def __init__(self, player, funds=100):

        self.dealer = Dealer()
        self.player = Player(player, funds)
        self.deck = Deck()

        # call table_setup() method to shuffle and deal first cards
        self.table_setup()

    def table_setup(self):

        # shuffle the deck when we all 'sit down' at the table before dealing
        self.deck.shuffle()

        # place initial bet for player
        self.player.place_bet()

        # deal a card to the player, then the dealer, then the player to start the game
        self.deal_card(self.player)
        self.deal_card(self.dealer)
        self.deal_card(self.player)
        self.calculate_score(self.player)  # calculate the player and dealer score at start to check for blackjack
        self.calculate_score(self.dealer)

        # call self.main() which is where we will set up the recurring hit/stick prompt and deal cards
        self.main()

    def main(self):

        while True:
            print()
            print(self)
            player_move = self.player.hit_or_stick()
            if player_move is True:
                self.deal_card(self.player)
                self.calculate_score(self.player)
            elif player_move is False:
                self.dealer_hit()

    def dealer_hit(self):

        score = self.dealer.score
        while True:
            if score < 17:
                self.deal_card(self.dealer)
                self.calculate_score(self.dealer)
                print(self)
            elif score >= 17:
                self.check_final_score()

    def __str__(self):  # this is just for checking progress during programming

        dealer_hand = [card for card, value in self.dealer.hand]
        player_hand = [card for card, value in self.player.hand]

        print("Dealer hand : {}".format(dealer_hand))
        print("Dealer score : {}".format(self.dealer.score))
        print()
        print("{}'s hand : {}".format(self.player.name, player_hand))
        print("{}'s score : {}".format(self.player.name, self.player.score))
        print()
        print(("{}'s current bet: {}.".format(self.player.name, self.player.bet)))
        print("{}'s current bank: {}.".format(self.player.name, self.player.funds))
        print("-" * 40)
        return ''

    def deal_card(self, player):

        card = self.deck.stack.pop()
        player.hand.append(card)

    def calculate_score(self, player):

        ace = False  # figure a way to check for ace in hand
        score = 0
        for card in player.hand:
            if card[1] == 1 and not ace:
                ace = True
                card = ('A', 11)
            score += card[1]
        player.score = score
        if player.score > 21 and ace:
            player.score -= 10
            score = player.score
        self.check_win(score, player)
        return

    def check_win(self, score, player):
        if score > 21:
            print()
            print(self)
            print("{} busts".format(player.name))
            print()
            self.end_game()
        elif score == 21:
            print(self)
            print("{} blackjack!".format(player.name))
            try:  # can only payout if player wins, not dealer.  Protecting with try / except
                player.payout()
            except:
                pass
            self.end_game()
        else:
            return

    def check_final_score(self):

        dealer_score = self.dealer.score
        player_score = self.player.score

        if dealer_score > player_score:
            print("Dealer wins!")
            self.end_game()
        else:
            print("{} wins!".format(self.player.name))
            self.end_game()

    def end_game(self):

        bank = self.player.funds
        if bank >=10:
            again = input("Do you want to play again (Y/N)? ")
            if again.lower().startswith('y'):
                self.__init__(self.player.name, funds=self.player.funds)
            elif again.lower().startswith('n'):
                exit(1)  # just trying exit code 1 to confirm this is exiting when I ask
        elif bank < 10:
            print("You're all out of money!  Come back with some more dough, good luck next time!")
            exit(2)


class Dealer(object):

    def __init__(self):

        self.name = "Dealer"
        self.score = 0
        self.hand = []


class Player(Dealer):

    def __init__(self, name, funds, bet=0):
        super().__init__()
        self.name = name
        self.funds = funds
        self.bet = bet

    def place_bet(self, amount=10):  # I might later incorporate a way to change amount, for now just default to 10

        # called at the beginning of every hand
        self.funds -= amount
        self.bet += amount

    def payout(self):

        # money is subtracted from funds at start of each hand when bet goes down
        # payout is 1:1 always (for now, maybe switch to 3:2 if player gets blackjack)
        self.funds += (self.bet * 2)
        self.bet = 0

    @staticmethod
    def hit_or_stick():
        while True:
            choice = input("Do you want another card (Y/N)? ")
            if choice.lower().startswith('y'):
                return True
            elif choice.lower().startswith('n'):
                return False
            else:
                print("I didn't understand")
                continue


class Deck(object):

    # using one stack for now
    # create a list of all the values and shuffle them
    # when dealing the cards use pop() to get the card off the top of the stack

    def __init__(self):

        # stack is composed of tuples:
        # [0] is a string to show the player for their hand
        self.stack = [('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)] * 4
        self.shuffle()

    def shuffle(self):

        shuffle(self.stack)

    def deal_card(self):

        card = self.stack.pop()
        return card


def main():

    player_name = input("Welcome to the casino!  What's your name? ")
    Table(player_name)


if __name__ == '__main__':

    main()
\$\endgroup\$
  • \$\begingroup\$ Complete aside, but OOP is not considered essential by everyone. This video is a pretty good introduction to some of the practical problems associated with it, even if you disagree with some or all of its conclusions. \$\endgroup\$ – jpmc26 Oct 9 '17 at 21:07
  • \$\begingroup\$ @jpmc26 thanks for sharing that video. I started it but didn't have a chance to finish it yet (I want to dedicate my attention when watching). I don't think I have enough experience to have an opinion yet, but I do read things often while going through my self-education like "Object Oriented Programming is the industry standard." I saw from the start of the video that this is addressed in his argument so I'll be glad to learn more about why. I guess I'm concerned that if I shy away from it my job prospects may be hindered in the future? But I want to learn as much as possible. \$\endgroup\$ – Hanzy Oct 10 '17 at 18:01
  • \$\begingroup\$ There's nothing wrong with studying OO and coming to your own conclusion. And as you importantly observe, following your team's conventions is often more important that picking a perfect paradigm. But it's good to be informed about the downsides and pitfalls of the paradigm, as well. Even though I have strong opinions about the ideal way to code, I try to hold to only one dogma about programming nowadays: "There are no silver bullets." Nothing replaces care, caution, and testing when you develop code. \$\endgroup\$ – jpmc26 Oct 14 '17 at 2:06
8
\$\begingroup\$

Few simple suggestions:

  1. In main() you can write:

    • if player_move: instead of if player_move is True: because they are equivalent.
    • else instead of elif player_move is False:, this makes more sense and is shorter.
  2. In dealer_hit(self), you can simply write else instead of elif score >= 17:, this is because when you have only two test cases then if ... else is more appropriate.

  3. You can remove the return statement whenever you write only return without a specific value, this is because in Python functions return, anyway, None when we do not specify a return value.

  4. I see you repeatedly leave a blank line between functions names and the first instruction of their bodies, but you should not. PEP8 does not ask to leave an empty line in that case.

  5. Some of your comments are quite long, so at least write them right before the instructions that they describe instead of writing them on the side. If a line exceeds 80 characters, you should break it into 2 or more shorter lines.

\$\endgroup\$
  • \$\begingroup\$ Thanks so much, I'll make those edits! I know 80 characters is recommended but I'm using IntelliJ and it lets me get away with 120 without yelling at me, XD. But I will stick to 80! \$\endgroup\$ – Hanzy Oct 9 '17 at 14:20
  • \$\begingroup\$ You are welcome. Any simple text editor will allow you to stretch your lines longer than 80 characters, but it is a PEP8 recommendation (I linked to PEP8 in my answer). Long lines are hard to follow, shorter lines help in readability and clarity. \$\endgroup\$ – Billal Begueradj Oct 9 '17 at 14:25
  • \$\begingroup\$ @BillalBEGUERADJ Beyond PEP8. The 80 char recommendation has more to do with the tradition of working at the command line (such as using tools like emacs and vim nowadays) where the screen may be limited to 80 characters than it does with readability. \$\endgroup\$ – jpmc26 Oct 9 '17 at 21:49
  • 2
    \$\begingroup\$ "if player_move: instead of if player_move is True: because they are equivalent" The recommendation is good, but they're only equivalent if you can guarantee that player_move is always a boolean. They behave differently if player_move could be a list, for instance. "when you have only two test cases then if ... else is more appropriate" It's only more appropriate if you have 2 mutally exclusive conditions; if you have unhandled cases, the explicit checks plus an error in a final else when neither is met is preferable. \$\endgroup\$ – jpmc26 Oct 9 '17 at 21:55
  • 1
    \$\begingroup\$ @BillalBEGUERADJ Having to rework your design in unexpected ways is not a sign of amateurism. It is often a sign of realizing your current design simply does not fit well with your new requirements, which you typically cannot foresee. I mentioned these issues because the OP seems to be a rather new, inexperienced developer, so it's important to be clear about what the limitations of your advice are to limit the risk of misunderstanding and misapplication. Especially early in one's career, there is a strong temptation to take advice to dogmatic and wrong extremes. \$\endgroup\$ – jpmc26 Oct 10 '17 at 6:32
4
\$\begingroup\$

On top of the comments provided by Billal BEGEURADJ, you also have the possibility of a stack recursion error in your code:

Table(...) calls Table.__init__(...)

Table.__init__(...) calls Table.table_setup()

Table.table_setup(...) calls Table.main(...)

Table.main(...) calls Table.calculate_score(...)

Table.calculate_score(...) calls Table.check_win(...)

Table.check_win(...) calls Table.end_game(...)

And Table.end_game(...) calls Table.__init__(...)

To fix this it may be worth making the game get started from outside of the class in a loop:

def main():
    player_name = input("Welcome to the casino!  What's your name? ")
    table = Table(player_name)
    player_continue = 'y'
    while player_continue in ['y', 'Y', 'yes', 'Yes']:
        table.play()
        player_continue = input("You ran out of money, do you want to go to the bank and come back (y/n)?")

if __name__ == '__main__':
    main()

Note that this would require a few changes to make it work but would result in there not being any recursion hidden away.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the heads up! I couldn't decide how to re-initialize the table... my goal was to get as "object oriented" as possible but I hadn't even considered a recursion error. Just so I understand, is it just Table.__init__ at the end of the loop that will be the recursive stack? Or does each function inside the Table get added to the stack also? Just curious if a person would need to play roughly 1000 times or far fewer before the limit is reached. \$\endgroup\$ – Hanzy Oct 9 '17 at 17:59
  • 1
    \$\begingroup\$ @Hanzy each one in turn, each loop of the game adds several functions to the recursive stack, you're probably looking at about 150 games before the error if your recursion stack size is set to 1000 call frames(which it is by default) \$\endgroup\$ – Nick A Oct 9 '17 at 18:02
  • \$\begingroup\$ Thanks, that's very helpful. So when trying to understand this, if I were to monitor the amount of memory being used by my program, I should be able to see it continually increase on every round of blackjack? Or maybe I'm misunderstanding how the recursion stack works (I have worked around it before but never really needed to inspect how it's implemented until now... I simply iterated the code instead, but now I'm trying to get a better understanding of what's at play). I'm trying to see if the variables inside Table get overwritten when calling Table.__init__ as well I suppose. \$\endgroup\$ – Hanzy Oct 9 '17 at 18:15
  • \$\begingroup\$ or rather, if the old references to variables in Table get deleted or stored based on the recursion? \$\endgroup\$ – Hanzy Oct 9 '17 at 18:17
  • 1
    \$\begingroup\$ @Hanzy you would indeed see the memory gradually got up as further stacks were allocated \$\endgroup\$ – Nick A Oct 9 '17 at 18:25
2
\$\begingroup\$

Smaller things I noticed

  • __str__ should not print but rather return a suitable ("informal") string representation of an object.

  • Don't (ab)use implementation details instead of accessing the interface: deal_card of Table directly accesses the stack of the Deck instead of using Deck's deal_card. Basically the same issue with calling player.hand.append in the same method.

  • try/except used for "normal" (non exceptional) control flow

  • Having __init__ do all the work seems wrong: It should setup an instance of the class, not more.

(No tests?)

Try to test your code as early as possible. Even more, at least according to TDD, you should write your tests first.

I'm still looking for the object orientation I was promised ...

Sorry to be the elephant in the room, but I don't see much of an object oriented design here. It's more like wrapping all the code in methods of some arbitrary classes. Let me try to explain:

  • You have a Dealer .. which does nothing.

  • A Player is-a Dealer? A Player can place a bet (sounds good) and can payout (probably not so good).

  • There's a Deck which can (be) shuffle(d). And it can deal a card.

  • Finally, there's the Table. It can setup itself, can create Player and Dealer and Deck, it can also deal cards, calculate some score and the final score, check for a win and also end the game. It's also doing the output and overall control over the game.

IMO this is a huge imbalance. The Table basically controls and does everything with the small exception that it's not shuffling the cards nor handle the player's bets / funds. As such, it's a violation of the SRP.


To Be Continued

\$\endgroup\$
  • \$\begingroup\$ Thank you, that's very helpful. Doing a lot of self education, I've found that many resources teach you syntactically how to write code, but skip some larger concepts that address implementation (like the ideas you pointed out). Already I can see how I would redesign things if I were to do it again (maybe I will, but not today - this is serving a good purpose as a learning exercise for me). I think that trying to keep all functions inside a class was a mistake, but I hadn't considered why it was a mistake. I was just overzealous in trying to apply that idea. \$\endgroup\$ – Hanzy Oct 10 '17 at 16:24
  • 1
    \$\begingroup\$ I'll try to come up with an explanation of how I'd approach an OO design here, hopefully in a few hours \$\endgroup\$ – Daniel Jour Oct 10 '17 at 16:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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