2
\$\begingroup\$

what do you think of my BlackJack game in regards in Object-Oriented Programming?

My code is at https://github.com/ngaisteve1/BlackJack
A follow-up question is available: Follow-Up

using System;
using System.Threading;

public class BlackJackGame
{
private static deckCard deck;
public void Play()
{
    bool continuePlay = true;

    Console.Title = "Steve BlackJack Game (Version 2)";
    Console.Write("Steve BlackJack Game ");
    Utility.MakeColor2(" ♠ ",ConsoleColor.White);
    Utility.MakeColor2(" ♥ ",ConsoleColor.Red);
    Utility.MakeColor2(" ♣ ",ConsoleColor.White);
    Utility.MakeColor2(" ♦ ",ConsoleColor.Red);

    deck = new deckCard();
    Console.Write("\n\nEnter player's name: ");

    // Create player
    var player = new Player(Console.ReadLine());

    // Create dealer
    var dealerComputer = new Player();

    while (continuePlay)
    {
        // Initialize screen and player's certain property - Start
        Console.Clear();
        player.IsNaturalBlackJack = false;
        player.IsBusted = false;
        dealerComputer.IsNaturalBlackJack = false;
        dealerComputer.IsBusted = false;
        // Initialize screen and player's certain property - End            

        if (deck.GetRemainingDeckCount() < 20)
        {
            // Get a new shuffled deck.
            deck.Initialize();
            Console.WriteLine("Low number of cards remaining. New cold deck created.");
        }

        deck.ShowRemainingDeckCount();

        // Show player bank roll
        Console.WriteLine($"{player.Name} Chips Balance: {player.ChipsOnHand}");

        // Get bet amount from player
        Console.Write("Enter chip bet amount: ");
        player.ChipsOnBet = Convert.ToInt16(Console.ReadLine());

        // Deal first two cards to player
        deck.DealHand(player);

        // Show player's hand
        player.ShowUpCard();
        Thread.Sleep(1500);

        // Deal first two cards to dealer
        deck.DealHand(dealerComputer);

        // Show dealer's hand        
        dealerComputer.ShowUpCard(true);
        Thread.Sleep(1500);
        // Check natural black jack
        if (!checkNaturalBlack(player, dealerComputer))
        {
            // If both also don't have natural black jack, 
            // then player's turn to continue.
            PlayerAction(player);

            Console.WriteLine("\n--------------------------------------------------");

            PlayerAction(dealerComputer);

            Console.WriteLine("\n--------------------------------------------------");

            //Announce the winner.
            AnnounceWinner(player, dealerComputer);
        }

        Console.WriteLine("This round is over.");

        Console.Write("\nPlay again? Y or N? ");

        continuePlay = Console.ReadLine() == "Y" ? true : false;
        // for brevity, no input validation
    }

    Console.WriteLine($"{player.Name} won {player.TotalWins} times.");
    Console.WriteLine($"{dealerComputer.Name} won {dealerComputer.TotalWins} times.");
    Console.WriteLine("Game over. Thank you for playing.");

}

private static void PlayerAction(Player currentPlayer)
{
    // set to player's turn

    bool playerTurnContinue = true;

    string opt = "";

    while (playerTurnContinue)
    {
        Console.Write($"\n{currentPlayer.Name}'s turn. ");

        if (currentPlayer.Name.Equals("Dealer"))
        {
            Thread.Sleep(2000); // faking thinking time.
            // Mini A.I for dealer.
            opt = currentPlayer.GetHandValue() < 16 ? "H" : "S";
        }
        else
        {
            // Prompt player to enter Hit or Stand.
            Console.Write("Hit (H) or Stand (S): ");
            opt = Console.ReadLine();
        }

        switch (opt.ToUpper())
        {
            case "H":
                Console.Write($"{currentPlayer.Name} hits. ");
                Thread.Sleep(1500);
                // Take a card from the deck and put into player's Hand.
                currentPlayer.Hand.Add(deck.DrawCard());
                Thread.Sleep(1500);

                // Check if there is any Ace in the Hand. If yes, change all the Ace's value to 1.
                if (currentPlayer.GetHandValue() > 21 && currentPlayer.CheckAceInHand())
                    currentPlayer.Hand = currentPlayer.ChangeAceValueInHand();

                currentPlayer.ShowHandValue();

                break;
            case "S":
                if (currentPlayer.GetHandValue() < 16)
                    Console.WriteLine($"{currentPlayer.Name} is not allowed to stands when hand value is less than 16.");
                else
                {
                    Console.WriteLine($"{currentPlayer.Name} stands.");
                    Thread.Sleep(1500);
                    // Show player's hand
                    currentPlayer.ShowUpCard();
                    Thread.Sleep(1500);
                    Console.WriteLine($"{currentPlayer.Name}'s turn is over.");
                    Thread.Sleep(1500);
                    playerTurnContinue = false;
                }

                break;
            default:
                Console.WriteLine("Invalid command.");
                break;
        }

        // If current player is busted, turn is over.
        if (currentPlayer.GetHandValue() > 21)
        {
            Utility.MakeColor("Busted!", ConsoleColor.Red);
            Thread.Sleep(1500);
            Console.WriteLine($"{currentPlayer.Name}'s turn is over.");
            Thread.Sleep(1500);
            currentPlayer.IsBusted = true;
            playerTurnContinue = false;
        }
        // If current player total card in hand is 5, turn is over.
        else if (currentPlayer.Hand.Count == 5)
        {
            Console.WriteLine($"{currentPlayer.Name} got 5 cards in hand already.");
            Thread.Sleep(1500);
            Console.WriteLine($"{currentPlayer.Name}'s turn is over.");
            Thread.Sleep(1500);
            playerTurnContinue = false;
        }


    }
}



private static bool checkNaturalBlack(Player _player, Player _dealer)
{
    Console.WriteLine();
    if (_dealer.IsNaturalBlackJack && _player.IsNaturalBlackJack)
    {
        Console.WriteLine("Player and Dealer got natural BlackJack. Tie Game!");
        _dealer.ShowUpCard();

        return true;
    }
    else if (_dealer.IsNaturalBlackJack && !_player.IsNaturalBlackJack)
    {
        Console.WriteLine($"{_dealer.Name} got natural BlackJack. {_dealer.Name} won!");
        _dealer.ShowUpCard();
        _dealer.AddWinCount();
        _player.ChipsOnHand = _player.ChipsOnHand - (int)Math.Floor(_player.ChipsOnBet * 1.5);            
        return true;
    }
    else if (!_dealer.IsNaturalBlackJack && _player.IsNaturalBlackJack)
    {
        Console.WriteLine($"{_player.Name} got natural BlackJack. {_player.Name} won!");
        _player.AddWinCount();
        _player.ChipsOnHand = _player.ChipsOnHand + (int)Math.Floor(_player.ChipsOnBet * 1.5);
        return true;
    }

    // guard block
    return false;
}

private static void AnnounceWinner(Player _player, Player _dealer)
{
    Console.WriteLine();
    if (!_dealer.IsBusted && _player.IsBusted)
    {
        Console.WriteLine($"{_dealer.Name} won.");
        _dealer.AddWinCount();
    }
    else if (_dealer.IsBusted && !_player.IsBusted)
    {
        Console.WriteLine($"{_player.Name} won.");
        _player.AddWinCount();
        _player.ChipsOnHand = _player.ChipsOnHand + _player.ChipsOnBet;
    }
    else if (_dealer.IsBusted && _player.IsBusted)
        Console.WriteLine("Tie game.");
    else if (!_dealer.IsBusted && !_player.IsBusted)
        if (_player.GetHandValue() > _dealer.GetHandValue())
        {
            Console.WriteLine($"{_player.Name} won.");
            _player.AddWinCount();
            _player.ChipsOnHand = _player.ChipsOnHand + _player.ChipsOnBet;
        }
        else if (_player.GetHandValue() < _dealer.GetHandValue())
        {
            Console.WriteLine($"{_dealer.Name} won.");
            _dealer.AddWinCount();
            _player.ChipsOnHand = _player.ChipsOnHand - _player.ChipsOnBet;                
        }

        else if (_player.GetHandValue() == _dealer.GetHandValue())
            Console.WriteLine("Tie game.");


}

}

\$\endgroup\$
  • 3
    \$\begingroup\$ Might help to include deckCard, card, Player, etc. \$\endgroup\$ – Shelby115 Feb 27 at 15:49
2
\$\begingroup\$

I have played a lot of Blackjack in my life and was looking for a little challenge when I came across your question.

So, first let me thank you for inspiring me to code a version of Windows Console Blackjack. I incorporated some of your code and ideas - like the console symbols and color changes, and the shuffle algorithm.

My version is here: https://github.com/lucidobjects/Blackjack

That's my basic take on how to model Blackjack in OOP. I invite you to play it and review the code. As you will see, I adhere to object-oriented principles, including preventing any object from directly setting the internals of any other object.

Regarding your code, here are some thoughts:

  1. You are definitely on the right track to think about building a Hand class.
  2. Because they have enormously different behaviors, combining Dealer and Player into a single class is a challenging road to take. I would strongly recommend splitting Dealer out to its own class (as I did).
  3. Your BlackJackGame class encompasses the functionality of a Casino, a Table, and a Dealer - all of which should be separate classes.
  4. You might want to look into encapsulating the console writing on the appropriate objects. All the classes that get written to the screen could have a public Draw() method.
  5. The Deck's DealHand method is another indication that there should be a Dealer class.
  6. For things like Deck.GetRemainingDeckCount() you might want to consider a Remaining property rather than a method. Though picking between a method and a property can be tricky, as I found out when coding mine. I came to the conclusion that if something has the attribute throughout its existence then it's a property. If it only has the attribute sometimes - like after the cards have been dealt, then more likely a method.
  7. If you are committed to C#, I recommend learning LINQ. It took me a while to even start with LINQ and a while more to learn the basics. But, it has definitely been worth the investment. Now I'm a huge fan of LINQ.
  8. Another C# feature you might want to investigate is expression-bodied members, which I use extensively.
  9. It's a matter of personal preference but I'm a devotee of var.
  10. I also avoid Utility classes as much as possible. In recent memory I have successfully avoided them entirely.
  11. I also typically avoid static but in the case of the Table class in my version, I figured that's the stuff that's written directly on the felt and/or a sign on the table, so I made an exception.If I were implementing a multi-table casino, that stuff would stop being static and probably move into a TableRules class.
\$\endgroup\$
  • \$\begingroup\$ wow, thanks a lot for all the comment and suggestion. gonna improve my BlackJack in the next version. \$\endgroup\$ – Steve Ngai Mar 17 at 19:52
  • \$\begingroup\$ You're welcome. I look forward to seeing the next version. Also, I moved my Blackjack repo and updated the link in my answer. \$\endgroup\$ – Aron Mar 18 at 2:52
5
\$\begingroup\$

This is by no means a full review, but I figured I'd mention some of the things I noticed real quick.

Naming & Consistency

What is a deckCard? Oh, you mean a DeckOfCards? Why not just name it what it is? Secondly, we PascalCase names of classes in C#. See Microsoft Casing Guidelines. Also, looking at your github the Filenames should match the class names (e.g. card.cs should be Card.cs) and you should generally try to restrict each class to its own file. Helps you stay organized when projects get larger.

Modularity & Intent

You could separate your console code into smaller functions with representative names of what they do.

Examples:

bool IsPlayerBust(Player player);
bool HasPlayerWon(Player player);
void Hit(Player player, DeckOfCards deck);
bool CanPlayerStand(Player player);
void Stand(Player player);

This would have two benefits.

  1. Your code will be broken up into smaller more manageable pieces.
  2. You'll be able to give meaningful names to those smaller pieces of code. For example, currentPlayer.GetHandValue() > 21 could be named IsPlayerBust() or added to player as a function currentPlayer.IsBust(). Doing it this way shows intent and gives meaning to your lines of code so they may be more easily read.

Humans read C#; Computers don't.

You seem to take this advice into account in some places and not in others. Try to be consistent in your coding habits. Hope these tips help.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot for the review. Okay gonna break up into smaller functions. \$\endgroup\$ – Steve Ngai Feb 28 at 0:57
2
\$\begingroup\$

From a Unit Testing perspective, I would say many of the hard-coded strings in your Console.WriteLine() calls should be retrieved from some function that generates those strings.

Also, it will be near impossible to unit test whether or not your switch/case for getting the user's input on Hit/Stand without having those be their own separate functions.

Each action/input/output should have it's own function that will either return something or set a property on your Player class in order to Unit Test properly. This will also increase readability throughout your code, and will make debugging far easier, as you'll likely only have to step through a function or two to find any problems rather than stepping through that massive PlayerAction function.

This next suggestion is purely personal choice, and maybe nit-picky, but this block here lacks consistency:

            if (currentPlayer.GetHandValue() < 16)
                Console.WriteLine($"{currentPlayer.Name} is not allowed to stands when hand value is less than 16.");
            else
            {
                Console.WriteLine($"{currentPlayer.Name} stands.");
                Thread.Sleep(1500);
                // Show player's hand
                currentPlayer.ShowUpCard();
                Thread.Sleep(1500);
                Console.WriteLine($"{currentPlayer.Name}'s turn is over.");
                Thread.Sleep(1500);
                playerTurnContinue = false;
            }

Your if statement doesn't use { }, but your else does.

\$\endgroup\$
  • \$\begingroup\$ Oh, yes. I completely forgot to mention that bracket inconsistency. Which is funny because that was the reason I decided to make an answer. Good catch. \$\endgroup\$ – Shelby115 Feb 27 at 18:42
  • \$\begingroup\$ thanks a lot. gonna amend it to make it easier to test. oh the if statement because there is only one line , so i omit the bracket \$\endgroup\$ – Steve Ngai Feb 28 at 0:58
  • \$\begingroup\$ Be consistent with your backets. If the else-statement has brackets it's more readable and expected for your if-statement to have brackets too, even if it's one line. It's completely fine to omit them if both are one line or if there is no else-statement though. \$\endgroup\$ – Shelby115 Feb 28 at 2:12
  • \$\begingroup\$ I have refactor my code accordingly already. Regarding the input and output separation, I do understand the benefit. It is just like MVC. But in Console project, I am still thinking how to separate it nicely. Is it like creating a class Screen for both input and output? \$\endgroup\$ – Steve Ngai Feb 28 at 10:47

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.