Sign in to follow this  
Jsin

Some useful advice would be nice.

Recommended Posts

Alright guy this is the deal. I have a taken two programming classes so far, one was just about the logic of programming no specific language was used. The other was an introduction in C# nothing advanced. My next class WAS going to be more advanced concepts of C#, but that class got canceled. It will be a while before I can take the second class. So I though in order to stay fresh on the things, I would write simple programs for card games I know. This program of high/low works, but what I want to know from you guys is this.

A) Are there any bad habits that I'm forming that aren't good practices for the professional world of programming?

B) Is there something that I did the hard way that could have been done much simpler?


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

namespace War4
{
class Program
{
static void Main(string[] args)
{
string playerName;
double cash = 250;
double betAmount = 0;
int player, dealer;
char more = 'y';
int win = 0;

Console.Write("Enter your name: ");
playerName = Console.ReadLine();

do
{
do
{
Console.WriteLine("You have {0:C} to bet with", cash);
betAmount = GetBetAmount(betAmount);
}
while (betAmount > cash);

Console.WriteLine("{0} press enter to draw a card", playerName);
Console.ReadLine();
player = GetCard();
DisplayCard(player);

Console.WriteLine("Press enter to see the dealers card");
Console.ReadLine();
dealer = GetCard();
DisplayCard(dealer);

win = Evaluate(player, dealer);

if (win == 3)
{
do
{
Console.WriteLine("TIE!! NOW WE GO TO WAR!!");
Console.WriteLine("{0} press enter to draw a card", playerName);
Console.ReadLine();
player = GetCard();
DisplayCard(player);

Console.WriteLine("Press enter to see the dealers card");
Console.ReadLine();
dealer = GetCard();
DisplayCard(dealer);

win = Evaluate(player, dealer);

}
while (win == 3);
}

if (win == 1)
{
Console.WriteLine("YOU WIN!!");
cash = cash + (betAmount * 2);
Console.WriteLine();
}
else
{
Console.WriteLine("YOU LOSE!!");
cash -= betAmount;
Console.WriteLine();
}

Console.Write("Press [Y] to continue to play: ");
more = Convert.ToChar(Console.ReadLine());
Console.WriteLine();

if (cash == 0)
{
Console.WriteLine("{0} have no more money",playerName);
more = 'n';
Console.WriteLine();
}
}
while (more == 'y' || more == 'Y');
Console.WriteLine("Thank you for playing.");
Console.WriteLine();

}


static double GetBetAmount(double betAmount)
{
Console.Write("Please enter bet amount: ");
betAmount = Convert.ToDouble(Console.ReadLine());
return betAmount;
}
static int GetCard()
{
Random rnd = new Random();
int card = rnd.Next(2, 14);
return card;
}

static void DisplayCard(int card)
{
if (card == 14)
{
Console.WriteLine("ACE");
Console.WriteLine();
}
else
if (card == 13)
{
Console.WriteLine("KING");
Console.WriteLine();
}
else
if (card == 12)
{
Console.WriteLine("QUEEN");
Console.WriteLine();
}
else
if (card == 11)
{
Console.WriteLine("JACK");
Console.WriteLine();
}
else
if (card >= 2 && card <= 10)
{
Console.WriteLine(card);
Console.WriteLine();
}
else
{
Console.WriteLine("Error {0} is invaid card!", card);
Console.WriteLine();
}
}

static int Evaluate(int player, int dealer)
{


if (player > dealer)
{
return 1;
}
else
if (player < dealer)
{
return 2;

}
else
{
return 3;
}
}

}
}




Your advice is greatly appreciated.

Share this post


Link to post
Share on other sites
For a beginner what you have is fine.



To grow, you will need to start to think in systems.

You have got several systems in your simple game, but I'll focus on just the most obvious one.

A deck of cards is a system. It is more than just a random number generator, but a collection of specific cards that gets shuffled, sorted, drawn from, and re-inserted to. Drawing a card will pull it from the deck or otherwise mark it as 'in use'. Shuffling cards will reorder those in the collection without modifying those outside it. A deck of cards contains data, and it isn't necessarily a poker deck. It could be a poker deck with jokers. It could be a double poker deck,a pinochle deck, double pinochle deck, or even cards for old maid.

If you allow replaceable cards you can use the same system for ANY card game. Even a deck of YuGiOh or Magic cards can be manipulated, since fundamentally they are the same. There can be multiple decks of cards for any purpose. Even a player's hand could be treated as a small deck of cards.

A deck of cards isn't just an instance of a standard container class. You've got some of the same operations like add, remove, and sort. Your standard container may have shuffle. But it does not have things like "cut", where you divide the deck at a single location and swap the two halves. These deck-specific operations can be added as you need them.

Ultimately you can reuse the deck of cards functionality any time you need a collection of cards in every other card game you develop, no matter the details.

This grows toward "data driven" programming, which is needed for any significant project.



When you start to think in systems it can take more effort than the small toy program you wrote. However, once you start adding parts that interact with each other and start adding game data that needs to be manipulated, a data driven system is much easier to write, maintain, and debug in the long run.

Share this post


Link to post
Share on other sites
Your use of "magic" numbers will get you into a bad habit.
Instead of your Evaluate() function returing strictly numbers, it should instead return values of defines or const ints... i.e.





#define EVALUATE_WIN 1
#define EVALUATE_LOSE 2
#define EVALUATE_TIE 3



static int Evaluate(int player, int dealer)
{


if (player > dealer)
{
return EVALUATE_WIN ;
}
else
if (player < dealer)
{
return EVALUATE_LOSE ;

}
else
{
return EVALUATE_TIE ;
}
}

Share this post


Link to post
Share on other sites
First off, instead of returning magic numbers like 1,2,3 in your evaluate. Use an enum. ie

enum { HAND_WIN, HAND_LOSE, HAND_DRAW };

You can do something similar with the cards, and map the enums into an array of strings to output the card names.

edit: beaten to the punch. Though I recommend against the macros. Enums are type safe, and will keep you from getting into trouble. Macros aren't type safe, and so you miss out on a lot of compiler checks that make sure the code is doing what you actually wanted it to do.

Share this post


Link to post
Share on other sites
Quote:

#define EVALUATE_WIN 1
#define EVALUATE_LOSE 2
#define EVALUATE_TIE 3

The code in the OP is C#, and these are C++ macros; they will not work in C#. "const int EVALUATE_WIN = 1" (as a member variable) is the near equivalent.

Share this post


Link to post
Share on other sites
I would get into the habit of putting comments in your code, especially around the more complex algorithms. From my own personal experience, I have often written code, confident that I know what it does, leave it for two weeks, come back to it and think "what the hell does this do?!" - believe me it is not a nice situation to be in, and it quickly wastes a lot of time.

Share this post


Link to post
Share on other sites
Quote:
Original post by jpetrie
Quote:

#define EVALUATE_WIN 1
#define EVALUATE_LOSE 2
#define EVALUATE_TIE 3

The code in the OP is C#, and these are C++ macros; they will not work in C#. "const int EVALUATE_WIN = 1" (as a member variable) is the near equivalent.


Yes thanks I am a C++ programmer.
I do like KulSeran's idea of enums.

Share this post


Link to post
Share on other sites
I'd work on reducing the size of your Main() function. This means more, smaller functions. Also if you do the same thing at the end of every branch of a conditional, move that action out of the branches after the conditional. Ex:

if (win == 1) {
Console.WriteLine("YOU WIN!!");
cash = cash + (betAmount * 2);
Console.WriteLine();
} else {
Console.WriteLine("YOU LOSE!!");
cash -= betAmount;
Console.WriteLine();
}

can be rewritten:

if (win == 1) {
Console.WriteLine("YOU WIN!!");
cash = cash + (betAmount * 2);
} else {
Console.WriteLine("YOU LOSE!!");
cash -= betAmount;
}
Console.WriteLine();

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this