patterncsharpMinor
First C# attempt: Blackjack
Viewed 0 times
blackjackfirstattempt
Problem
I am learning c# and trying to wrap my head around OOP. This blackjack code is my first attempt. It works well enough, but it is probably terrible. Please let me know what to work on.
```
using System;
using System.Collections.Generic;
public class Program
{
public static void Main()
{
bool playGame = true;
int playerWins = 0;
int dealerWins = 0;
while (playGame)
{
bool hit = true;
bool getResult = true;
int getValues;
Dealer dealer = new Dealer();
Player player = new Player();
while (hit)
{
player.AddCard();
Console.Clear();
if (player.CountValues(out getValues))
{
getResult = false;
++dealerWins;
break;
}
Console.WriteLine("Dealer shows one of his cards:");
dealer.DisplayHand(0);
Console.WriteLine("\nYour hand:");
player.DisplayHand();
Console.WriteLine("\nWhat will you do?");
Console.WriteLine("(1) Hit");
Console.WriteLine("(2) Stay");
hit = userInput();
}
if (getResult)
{
player.CountValues(out getValues);
Console.Clear();
if (dealer.CountValues() >= getValues)
{
Console.WriteLine("Dealer wins");
Console.WriteLine("Dealers hand:");
dealer.DisplayHand();
Console.WriteLine("\nDealer Points: {0}",dealer.CountValues());
Console.WriteLine("\nYour hand:");
player.DisplayHand();
Console.WriteLine("\nPlayer Points: {0}",getValues);
++dealerWins;
}
else
```
using System;
using System.Collections.Generic;
public class Program
{
public static void Main()
{
bool playGame = true;
int playerWins = 0;
int dealerWins = 0;
while (playGame)
{
bool hit = true;
bool getResult = true;
int getValues;
Dealer dealer = new Dealer();
Player player = new Player();
while (hit)
{
player.AddCard();
Console.Clear();
if (player.CountValues(out getValues))
{
getResult = false;
++dealerWins;
break;
}
Console.WriteLine("Dealer shows one of his cards:");
dealer.DisplayHand(0);
Console.WriteLine("\nYour hand:");
player.DisplayHand();
Console.WriteLine("\nWhat will you do?");
Console.WriteLine("(1) Hit");
Console.WriteLine("(2) Stay");
hit = userInput();
}
if (getResult)
{
player.CountValues(out getValues);
Console.Clear();
if (dealer.CountValues() >= getValues)
{
Console.WriteLine("Dealer wins");
Console.WriteLine("Dealers hand:");
dealer.DisplayHand();
Console.WriteLine("\nDealer Points: {0}",dealer.CountValues());
Console.WriteLine("\nYour hand:");
player.DisplayHand();
Console.WriteLine("\nPlayer Points: {0}",getValues);
++dealerWins;
}
else
Solution
this._name = "Ace";Don't store information in strings. What happens if you want to check this value and type
if (card.Name == "Ave")? Or what about if (card.Name == "ace")? Those checks will both fail. Create an enum with these values.public enum Card
{
Two,
Three,
// ...
King,
Ace
}Now, if you have a card:
Card card = Card.Ace;You can print the name like this:
Console.WriteLine(card.ToString()); // the .ToString() might be optional--don't remember offhandAdditionally, you can now simplify that massive
switch statement somewhat:var rnd = new Random();
var card = (Card)rnd.NextInt(0, 13);
var cardValue = GetValue(card); // this method should switch over the enum value and return the appropriate value as shown below:
private int GetValue(Card card)
{
switch (card)
{
case Card.Two:
return 2;
case Card.Three:
return 3;
// ...
case Card.Ten:
case Card.Jack:
case Card.Queen:
case Card.King:
case Card.Ace:
return 10;
}
}Don't put your logic in
Main(). Main()'s job is to start and stop the program--not to do any logic. Your Main() should look something like this:static void Main()
{
PlayGame();
Console.ReadKey(); // keep window from closing automatically
}Look at all this duplication:
if (dealer.CountValues() >= getValues)
{
Console.WriteLine("Dealer wins");
Console.WriteLine("Dealers hand:");
dealer.DisplayHand();
Console.WriteLine("\nDealer Points: {0}",dealer.CountValues());
Console.WriteLine("\nYour hand:");
player.DisplayHand();
Console.WriteLine("\nPlayer Points: {0}",getValues);
++dealerWins;
}
else
{
Console.WriteLine("Player wins");
Console.WriteLine("Dealers hand:");
dealer.DisplayHand();
Console.WriteLine("\nDealer Points: {0}",dealer.CountValues());
Console.WriteLine("\nYour hand:");
player.DisplayHand();
Console.WriteLine("\nPlayer Points: {0}",getValues);
++playerWins;
}The only two different lines are the first and the last. Those two lines, and only those two lines, should be separated. This is much shorter, and therefore easier to understand, debug, update, and maintain, and does the same thing:
if (dealer.CountValues() >= getValues)
{
Console.WriteLine("Dealer wins");
++dealerWins;
}
else
{
Console.WriteLine("Player wins");
++playerWins;
}
Console.WriteLine("Dealers hand:");
dealer.DisplayHand();
Console.WriteLine("\nDealer Points: {0}",dealer.CountValues());
Console.WriteLine("\nYour hand:");
player.DisplayHand();
Console.WriteLine("\nPlayer Points: {0}",getValues);Code Snippets
this._name = "Ace";public enum Card
{
Two,
Three,
// ...
King,
Ace
}Card card = Card.Ace;Console.WriteLine(card.ToString()); // the .ToString() might be optional--don't remember offhandvar rnd = new Random();
var card = (Card)rnd.NextInt(0, 13);
var cardValue = GetValue(card); // this method should switch over the enum value and return the appropriate value as shown below:
private int GetValue(Card card)
{
switch (card)
{
case Card.Two:
return 2;
case Card.Three:
return 3;
// ...
case Card.Ten:
case Card.Jack:
case Card.Queen:
case Card.King:
case Card.Ace:
return 10;
}
}Context
StackExchange Code Review Q#158335, answer score: 7
Revisions (0)
No revisions yet.