Console Blackjack game

What do you think the weak points are and how can I improve? I thought I could really use a Hand class in the dealHand method.

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

namespace ConsoleBlackjack
{
public enum Suit
{
Heart,
Diamond,
Spade,
Club
}

public enum Face
{
Ace,
Two,
Three,
Four,
Five,
Six,
Seven,
Eight,
Nine,
Ten,
Jack,
Queen,
King,
}

public class Card
{
public Suit Suit { get; set; }
public Face Face { get; set; }
public int Value { get; set; }
}

public class Deck
{
private List cards;

public Deck()
{
this.Initialize();
}

public void Initialize()
{
cards = new List();

for (int i = 0; i 1)
{
n--;
int k = rng.Next(n + 1);
Card card = cards[k];
cards[k] = cards[n];
cards[n] = card;
}
}

public Card DrawACard()
{
if (cards.Count userHand;
static List dealerHand;

static void Main(string[] args)
{
Console.Title = "♠♥♣♦ Blackjack Game by Niv Harel";
Console.WriteLine("Welcome to blackjack 0.1a\n");

chips = 100;
deck = new Deck();
deck.Shuffle();

while (chips > 0)
{
DealHand();
Console.WriteLine("\nPress any key for the next hand...\n");
ConsoleKeyInfo userInput = Console.ReadKey(true);
}

Console.WriteLine("You Lost! see you next time...");
Console.ReadLine();
}

static void DealHand()
{
if (deck.GetAm

Is unshuffled deck ever useful for anything? If not, you might want to call Shuffle() at the end of Initialize(), which will also avoid having to call Shuffle() after Initialize(), which happens several times in your code.

cards.Add(new Card() { Suit = (Suit)i, Face = (Face)j });

if (j <= 8)
    cards[cards.Count - 1].Value = j + 1;
else 
    cards[cards.Count - 1].Value = 10;

You should use the Card object initializer to set Value too. Using Math.Min(), that could look like this:

cards.Add(new Card { Suit = (Suit)i, Face = (Face)j, Value = Math.Min(j + 1, 10) });

Card cardToReturn = cards[cards.Count - 1];
cards.RemoveAt(cards.Count - 1);
return cardToReturn;

This looks like you don't want List, you want Stack. With that, all this code would simplify to:

return cards.Pop();

public int GetAmountOfRemainingCrads()

Very simple Get methods are usually better as properties.

static void Main(string[] args)

When you're not using args, just remove that parameter:

static void Main()

ConsoleKeyInfo userInput = Console.ReadKey(true);

When you're not using the output of a function, you don't need to declare a variable for it:

Console.ReadKey(true);

Console.ReadLine().Trim().Replace(" ","")

What's the logic behind this? Why are you accepting strings like 10 0? And if you did intend that, what's the reason for that Trim()? Replace() will already remove all spaces, including those at the start and end of the string and I don't think any other whitespace characters (which is what the parameterless overload of Trim() removes) make sense here.

foreach (Card card in userHand)
{
    if (card.Face == Face.Ace)
    {
        card.Value += 10;
        break;
    }
}

I don't think your logic for handling aces is sufficient. If I understand the rules correcty, even an ace that's not among the first two cards can be counted as 11 and even ace that is among the first two can be counted as 1.


I thought I could really use a Hand class in the dealHand method.

I agree, there is enough repeated code to warrant a separate class for this.

bool insurance = false; ;

What does the second semicolon do here? Get rid of it.

//chips -= betAmount / 2;

Don't leave commented out code from previous versions of your code behind. After you know the commented out code is no longer useful, remove it completely.

if (userHand[0].Value + userHand[1].Value == 21 && insurance)
{
    amountLost = betAmount / 2;
    chips -= betAmount / 2;
}
else if (userHand[0].Value + userHand[1].Value != 21 && !insurance)
{
    amountLost = betAmount + betAmount / 2;
    chips -= betAmount + betAmount / 2;
}

You're already computing amountLost to display it, you should also use it to modify chips, since it will make your code more DRY:

if (userHand[0].Value + userHand[1].Value == 21 && insurance)
{
    amountLost = betAmount / 2;
}
else if (userHand[0].Value + userHand[1].Value != 21 && !insurance)
{
    amountLost = betAmount + betAmount / 2;
}

chips -= amountLost;

foreach (Card card in userHand)
{
    totalCardsValue += card.Value;
}

This could be simplified using LINQ:

int totalCardsValue = userHand.Sum(card => card.Value);

default:
    break;

This is like adding else { } after an if; it doesn't do anything, remove it.

Console.ReadLine();

Why is this here? How is the user supposed to know they have to press Enter here?

StackExchange Code Review Q#60314, answer score: 8