Command line Blackjack

Question

I'm a C# beginner and this is my try at making a command line Blackjack. I'm aware that the Dealer logic is still far from done, but I wanted to post my code for review.

I want to make the code as efficient as possible. Also, I want it to be readable, so that working with it after pauses is easier. I think it's quite readable already, but I know some of you can give me hints to make it better.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace CliBlackjackV2
{
    class Program
    {

        private static Random randomizer = new Random();
        private static bool gameLoop = true;
        private static bool cardDraw = true;


        enum cardColor
        {
            Spades,
            Hearths,
            Clubs,
            Diamonds,
        };

        enum cardFace
        {
            Ace,
            Two,
            Three,
            Four,
            Five,
            Six,
            Seven,
            Eight,
            Nine,
            Ten,
            Jack,
            Quen,
            King,
        };

        static void Main(string[] args)
        {
            while (gameLoop)
            {
                game();
                Console.Write("\n Play again? [Y/N]: ");
                string choice = Console.ReadLine().ToLower();
                switch (choice)
                {
                    case "n":
                        gameLoop = false;
                        break;
                    default:
                        break;
                }
            }
        }

        static void game()
        {
            //isl is the shortform for integer string list
            //since im treating the Faces as ints and the Collors as String (at least in my head)
            List<KeyValuePair<cardFace, cardColor>> islPlayerHand = new List<KeyValuePair<cardFace, cardColor>>();
            List<KeyValuePair<cardFace, cardColor>> islDealerHand = new List<KeyValuePair<cardFace, cardColor>>();

            islPlayerHand = getCards(islPlayerHand);
            islPlayerHand = getCards(islPlayerHand);

            islDealerHand = getCards(islDealerHand);
            islDealerHand = getCards(islDealerHand);

            renderBoard(islPlayerHand, islDealerHand);

            cardDraw = true;
            while (cardDraw)
            {
                cardDraw = playerCardDraw(islPlayerHand);
                renderBoard(islPlayerHand, islDealerHand);
                checkHand("Player", islPlayerHand);
            }

            while (true)
            {
                int dealerTotal = calcHandTotal(islDealerHand);
                while (dealerTotal < 16)
                {
                    islDealerHand = getCards(islDealerHand);
                    renderBoard(islPlayerHand, islDealerHand);
                    checkHand("dealer", islDealerHand);
                    dealerTotal = calcHandTotal(islDealerHand);
                }
                if (dealerTotal >= 16)
                {
                    break;
                }

            }

            renderBoard(islPlayerHand, islDealerHand);
            checkHand("Player", islPlayerHand);
            checkHand("dealer", islDealerHand);
            decideWinner(islPlayerHand, islDealerHand);
        }

        static void checkHand(string player, List<KeyValuePair<cardFace, cardColor>> hand)
        {
            int total = calcHandTotal(hand);

            if (total == 21)
            {
                Console.WriteLine($"\n {player} got a Blackjack!\n");
                cardDraw = false;
            }
            else if (total > 21)
            {
                Console.WriteLine($"\n {player} Busted!\n");
                cardDraw = false;

            }
        }

        static bool playerCardDraw(List<KeyValuePair<cardFace, cardColor>> playerHand)
        {
            bool cardDraw = true;
            Console.Write(" Would you like to (H)it or to (S)tand: ");
            string choice = Console.ReadLine().ToLower();

            switch (choice)
            {
                case "h":
                    playerHand = getCards(playerHand);
                    break;

                case "s":
                    cardDraw = false;
                    break;

                default:
                    break;
            }
            return cardDraw;
        }

        static int calcHandTotal(List<KeyValuePair<cardFace, cardColor>> hand)
        {
            int handTotal = 0;
            int aceCount = 0;

            foreach (KeyValuePair<cardFace, cardColor> card in hand)
            {

                if ((int)card.Key >= 10)
                {
                    handTotal += 10;
                }
                else if (card.Key == cardFace.Ace)
                {
                    aceCount++;
                }
                else
                {
                    handTotal += (int)card.Key + 1;
                }

            }

            //calculating the ace's Values properly
            if (aceCount > 0)
            {
                if (handTotal <= (11 - aceCount))
                {
                    handTotal += (aceCount - 1) + 11;
                }
                else
                {
                    handTotal += aceCount;
                }
            }
            return handTotal;
        }

        static void decideWinner(List<KeyValuePair<cardFace, cardColor>> playerHand, List<KeyValuePair<cardFace, cardColor>> dealerHand)
        {
            int hand1Total=calcHandTotal(playerHand);
            int hand2Total=calcHandTotal(dealerHand);

            if ((hand1Total > 21) && (hand2Total > 21))
            {
                Console.WriteLine(" Both busted!");
            }
            else if (hand1Total > 21)
            {
                Console.WriteLine(" Dealer Wins!");
            }
            else if (hand2Total > 21)
            {
                Console.WriteLine(" Player Wins!");
            }
            else if (hand1Total > hand2Total)
            {
                Console.WriteLine(" Player Wins!");
            }
            else if (hand2Total > hand1Total)
            {
                Console.WriteLine(" Dealer Wins!");
            }
            else
            {
                Console.WriteLine(" It's a tie!");
            }
        }

        static void renderBoard(List<KeyValuePair<cardFace, cardColor>> playerHand, List<KeyValuePair<cardFace, cardColor>> dealerHand)
        {
            Console.Clear();
            Console.WriteLine("***********************\n" +
                "*      PlayerHand     *\n" +
                "***********************\n");
            printCards(playerHand);
            Console.WriteLine($" Total: {calcHandTotal(playerHand)}\n");

            Console.WriteLine("***********************\n" +
                "*      DealerHand     *\n" +
                "***********************\n");
            printCards(dealerHand);
            Console.WriteLine($" Total: {calcHandTotal(dealerHand)}\n");
        }

        static void printCards(List<KeyValuePair<cardFace, cardColor>> hand)
        {
            int cardCount = 1;
            foreach (var card in hand)
            {
                Console.WriteLine($" #{cardCount}  {card.Key} of {card.Value}");
                cardCount++;
            }
            Console.WriteLine("");
        }

        static List<KeyValuePair<cardFace, cardColor>> getCards(List<KeyValuePair<cardFace, cardColor>> hand)
        {
            hand.Add(new KeyValuePair<cardFace, cardColor>((cardFace)randomizer.Next(0, 12), (cardColor)randomizer.Next(0, 3)));
            return hand;
        }
    }
}

Answer 1

Instead of

switch (choice)
{
    case "n":
        gameLoop = false;
        break;
    default:
        break;
}

you could use something like the following:

gameLoop = choice != "n";

or if you want to ignore cases (which makes sense for this case):

gameLoop = choice.ToUpper() != "N";
// or
gameLoop = !choice.Equals("n", StringComparison.InvariantCultureIgnoreCase)

---

//isl is the shortform for integer string list
//since im treating the Faces as ints and the Collors as String (at least in my head)
List<KeyValuePair<cardFace, cardColor>> islPlayerHand = new List<KeyValuePair<cardFace, cardColor>>();
List<KeyValuePair<cardFace, cardColor>> islDealerHand = new List<KeyValuePair<cardFace, cardColor>>();

• Consider using another struct / class for combining cardFace and cardColor instead of using a key value pair.
• Do not use any abbreviations that have to be explained with comments.

e.g.

struct Card
{
    public cardFace Face {get; set; }
    public cardColor Color {get; set; }
}

List<Card> cardsPlayer = new List<Card>();
List<Card> cardsDealer = new List<Card>();

---

Instead of using procedural programming, you could structure your code more object oriented.

A first step could be, to encapsulate the "game code" in it's own 'Game' class. Than you could define the cards for the dealer / player as instance variables and define methods that work on these instance variables instead of passing it to (nearly) all methods.

After that, you could try to create a Player class that contains all the player related code instead of using a list of cards for the player.

The resulting code becomes more readable and maintainable because code that belongs together is encapsulated in class and will be hidden by other parts of the program.

The usage of your new structured code could look like:

var game = new Game();
var game.Start();
// or

var player = new Player("Payer's name");
player.DrawCard();
player.DrawCard();
player.CheckHand();
// ...

Answer 2

Don't put all of your code in Program.cs. IMHO that file's Main method should basically do one thing: call a method in a class you wrote yourself.

---

Follow naming guidelines. Classes, enum names, methods should be PascalCase, for instance.

---

Don't use a Hungarian notation, especially not one of your own invention:

isl is the shortform for integer string list

Your IDE should tell you what type a variable is, not its name.

---

If there are strings you use repeatedly, e.g. "Player" and "dealer", make them const.

Answer 3

while (true)
{
    int dealerTotal = calcHandTotal(islDealerHand);
    while (dealerTotal < 16)
    {
        islDealerHand = getCards(islDealerHand);
        renderBoard(islPlayerHand, islDealerHand);
        checkHand("dealer", islDealerHand);
        dealerTotal = calcHandTotal(islDealerHand);
    }
    if (dealerTotal >= 16)
    {
        break;
    }
}

can be rewritten as just

int dealerTotal = calcHandTotal(islDealerHand);
while (dealerTotal < 16)
{
    islDealerHand = getCards(islDealerHand);
    renderBoard(islPlayerHand, islDealerHand);
    checkHand("dealer", islDealerHand);
    dealerTotal = calcHandTotal(islDealerHand);
}

(that is, removing the outer while-loop and the if-statement)

This is because after the inner loop exits, you know that dealerTotal < 16 must not be true (otherwise, the inner loop would keep going), and thus dealerTotal >= 16 is true. So the body of the if-statement will be run in every iteration of the outer loop. Thus, the first iteration of the outer loop will result in the break; statement executing, meaning that the outer loop will only ever run once.

Answer 4

The terms are suit and rank. Should have a card class. You do too much logic about a card in the flow code. Should not calcHandTotal(). Hand should just have a public property for the total. Random cards on the fly is bad. You could have duplicate cards in a hand. Need a deck and a proper shuffle. Player class with name and hand so you can add in bet later.

Answer 5

In decideWinner(), why not name the variables playerHandTotal and dealerHandTotal, following the very descriptive convention of its arguments, instead of using the confusing hand1Total and hand2Total?