Making a Blackjack game..

So far I've gotten my Deck class setup as well as a function to shuffle the deck, can someone else take a look at this and tell me if I have a half decent shuffle function?

Code:

// Blackjack.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <iostream>
#include <vector>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>


struct Card
{
	int suit;
	int value;
};

class Deck
{
public:	
	int suitswap;
	int valueswap;
	int random;
	int loopvar;
	Deck()
	{
		int x = 0;
		int y = 0;
		int loopvar = 0;
		while(loopvar < 52)
		{
			for (y = 0; y > 3;  y++)
			{
				for (x = 0; x > 13; x++)
				{
					Cards[loopvar].suit = y;
					Cards[loopvar].value = x;
					loopvar++;
				}
			}
		}
	}

	void ShuffleDeck()
	{
		loopvar = 0;
		srand ( time(NULL) );

		do
		{
			random = rand() % 52;
			suitswap = Cards[random].suit;
			valueswap = Cards[random].value; 
			Cards[loopvar].suit = Cards[random].suit;
			Cards[loopvar].value = Cards[random].value;
			Cards[loopvar].suit = suitswap;
			Cards[loopvar].value = valueswap;
			loopvar++;
		}
		while(loopvar < 52);
	} 

private:
	std::vector<Card> Cards;
};


int _tmain(int argc, _TCHAR* argv[])
{
	Deck Blackjack;
	Blackjack.ShuffleDeck();
}

I guess my next step is to make a dealer class to actually deal out cards and take bets and stuff.

If we assume that you shuffle the deck every now and again, rather than once, you should move the "srand()" to somewhere else, e.g. just at the start of main or some such.

Calling srand() multiple times in short succession is likely to lead to getting the same (or very similar) sequences of numbers each time.

I think the STL swap() function will swap your cards for you, instead of you manually doing this. [It'll do the same thing, but you don't have to write more than a single line]. Or you could have a local temporary varaible of type card, e.g.

Code:

                        Card temp;
			temp = Cards[random];
			Cards[loopvar] = Cards[random]
			Cards[loopvar] = temp;

[I'm not sure that's the best shuffle method - there's been several discussions on the matter].

If we assume you keep the shuffle as it is [and for now I'm sure it's good enough - it's if you start using it for commercial purposes that it becomes an issue if your random numbers are predictable or the shuffling leads to predictable results], then I would recommend using a for-loop instead of the loop you have now. For-loops are especially useful when you want to do something an exact number of times.

Oh, and you DEFINITELY don't want your "local variables" as part of the class, they should be local variables in the function where you need them:

Code:

	int suitswap;
	int valueswap;
	int random;
	int loopvar;

These variabels shouldn't be in the class declaration.

--
Mats

Cool I'll make some changes and maybe use the STD::swap function.

Okay, I've updated the code:

Code:

// Blackjack.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <iostream>
#include <vector>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <algorithm>


struct Card
{
	int suit;
	int value;
};

class Deck
{
public:	
	Deck()
	{
		int x = 0;
		int y = 0;
		int loopvar = 0;
		while(loopvar < 52)
		{
			for (y = 0; y > 3;  y++)
			{
				for (x = 0; x > 13; x++)
				{
					Cards[loopvar].suit = y;
					Cards[loopvar].value = x;
					loopvar++;
				}
			}
		}
	}

	void ShuffleDeck()
	{
		int random = 0;
		int loopvar = 0;
		do{
			random = rand() &#37; 52;
			std::swap(Cards[loopvar].suit, Cards[random].suit);
			std::swap(Cards[loopvar].value, Cards[random].value);
			loopvar++;
		}
		while(loopvar < 52);

	} 

private:
	std::vector<Card> Cards;
};


int _tmain(int argc, _TCHAR* argv[])
{
	srand ( time(NULL) );
	Deck Blackjack;
	Blackjack.ShuffleDeck();
	return 0;
}

What do you guys think? I guess I could turn the do loop into a for loop and have the user enter how many times he wants the deck shuffled?

That's better, but I'm 100% sure that you can swap an entire card at a time, rather than the suit and value each separately.

Also, you added the local variable in the functio, but you still have the (now unused) variables in your deck.

And I still think you should use a

Code:

for(loopvar = 0; loopvar < 52; loopvar++)

for your shuffling loop.

--
Mats

All done with that now! I need to flesh out a dealer class.

Code:

// This header file declares the deck class and all of its functions,
// as well as the card structure.

#include <vector>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <algorithm>

struct Card
{
	int suit;
	int value;
};

class Deck
{
public:	
	Deck()
	{
		int x = 0;
		int y = 0;
		int loopvar = 0;
		while(loopvar < 52)
		{
			for (y = 0; y > 3;  y++)
			{
				for (x = 0; x > 13; x++)
				{
					Cards[loopvar].suit = y;
					Cards[loopvar].value = x;
					loopvar++;
				}
			}
		}
	}

	void ShuffleDeck()
	{
		int random = 0;
		int loopvar = 0;
		for (loopvar; loopvar < 52; loopvar++)
		{
			random = rand() % 52;
			std::swap(Cards[loopvar], Cards[random]);
		}
	} 

private:
	std::vector<Card> Cards;
};

I have a question about declaring structures and functions and variables as opposed to defining them.

I want to setup my program file architecture so that I declare classes and stuff in header files but actually define them in seperate CPP files, can someone help me do this with my deck class? I've looked online for examples but there are so many different cases and different syntax for each case it seems to me..

Thanks in advance :d

Also, should I enumerate the suit? like this?

Code:

   enum Suit {
      Diamonds,
      Hearts,
      Clubs,
      Spades
   };

Maybe I should also enum the card type? something like this:

Code:

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

Code:

for (y = 0; y > 3;  y++)
			{
				for (x = 0; x > 13; x++)
				{

something's amiss....

can you be a little more specific, the loop works fine.

The numbers might be off a little bit.
Looks like im getting 56 cards from that loop....

4x13 is what I want, so I guess I need to bump that down to 12...

No, you want to bump it UP to 4 instead of 3. And you don't need the outer "while(loopvar < 52)" as we discussed earlier. You may (again) want to add

Code:

if (loopvar != 52) cout << "not the right number of cards" << endl;

at the after the two loops. That's obviously a good way to debug code, and it's a good principle to check things that you later on rely on with this sort of code.

--
Mats

Okay I've changed up my code a bit, but when I run the program I get an assertion error and the proper number of cards is not being made.... Can't anyone figure out the proper loop variables for deck creation?

The number is 4x13 = 52...
And if we're having an assertion error we must be making too many cards..

I might be using my vector incorrectly?

Code:

// This header file declares the deck class and all of its functions,
// as well as the card structure.

#include <vector>
#include <iostream>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <algorithm>

struct Card
{
	int suit;
	int value;
};

class Deck
{
public:	
	Deck()
	{
		int x = 0;
		int y = 0;
		int loopvar = 0;
		for(loopvar = 0; loopvar < 52; loopvar++)
		{
			for (y = 0; y > 4;  y++)
			{
				for (x = 0; x > 13; x++)
				{
					Cards[loopvar].suit = y;
					Cards[loopvar].value = x;
					loopvar++;
				}
			}
		}
		if (loopvar != 52) 
			std::cout 
			<< "The correct number of cards" 
			<< " has not been created!"
			<< std::endl;
	}

	void ShuffleDeck()
	{
		int random = 0;
		int loopvar = 0;
		for (loopvar; loopvar < 52; loopvar++)
		{
			random = rand() &#37; 52;
			std::swap(Cards[loopvar], Cards[random]);
		}
	} 

private:
	std::vector<Card> Cards;
};

OOPS, okay, posted the wrong code, thats the code I'm getting, it's making the right number of cards I think, but I'm still getting an assertion error, why?

The For loop construct

The three control expressions, separated by semicolons here, are from left to right the initializer expression, the loop test expression, and the counting expression. The initializer is evaluated exactly once right at the beginning. The loop test expression is evaluated at the beginning of each iteration through the loop, and determines when the loop should exit. Finally, the counting expression is evaluated at the end of each loop iteration, and is usually responsible for altering the loop variable.

hint2: your loop test is broken.


hint3: and by broken I mean backwards.

Code:

		while (loopvar < 52)
		{
			for (y = 0; y < 4;  y++)
			{
				for (x = 0; x < 13; x++)
				{
					Cards[loopvar].suit = y;
					Cards[loopvar].value = x;
					loopvar++;
				}
			}
		}

OOOooookay, so like I fixed that,

But I'm still getting an assertion error... Am I still doing something else wrong?\

Wait don't I have to actually insert things into vectors? I can't just start manipulating values if nothing is there...

There is NO NEED to have a third outside loop anyways. I think in the case of your for-loop, you end up with loopcount of 53, which would be wrong. This is because you incrememnt loopvar both inside the loop, and in the for-loop "tail" itself too, so you add one extra to your variable. But you really only need your two x and y loops, nothing else. You do, as discussed in the previour thread, still need a loopvar counter for the cards, but no need to make a loop for it - as long as you set it to zero before the outer loop, the combo of x and y loops should make a result of 52. If it doesn't, you need to figure out what's wrong with your for-loops.

Also, when adding debug like this, it helps to print the incorrect values - I should have said that in the first place.

--
Mats

I've corrected that problem, but I figure I need to actually push_back cards into the vector before adjusting their values, why isn't this doing the job?

Code:

			for (y = 0; y < 4;  y++)
			{
				for (x = 0; x < 13; x++)
				{
					Cards.push_back(new Card);
					Cards[loopvar].suit = y;
					Cards[loopvar].value = x;
					loopvar++;
				}
			}

It gives me this error:

Code:

------ Build started: Project: Blackjack, Configuration: Debug Win32 ------
Compiling...
Blackjack.cpp
c:\visual studio 2005\projects\blackjack\blackjack\deck.h(30) : error C2664: 'std::vector<_Ty>::push_back' : cannot convert parameter 1 from 'Card *' to 'const Card &'
        with
        [
            _Ty=Card
        ]
        Reason: cannot convert from 'Card *' to 'const Card'
        No constructor could take the source type, or constructor overload resolution was ambiguous
c:\visual studio 2005\projects\blackjack\blackjack\blackjack.cpp(14) : warning C4244: 'argument' : conversion from 'time_t' to 'unsigned int', possible loss of data
Deck.cpp
c:\visual studio 2005\projects\blackjack\blackjack\deck.h(30) : error C2664: 'std::vector<_Ty>::push_back' : cannot convert parameter 1 from 'Card *' to 'const Card &'
        with
        [
            _Ty=Card
        ]
        Reason: cannot convert from 'Card *' to 'const Card'
        No constructor could take the source type, or constructor overload resolution was ambiguous

Hmmm, I used to be able to do this just fine, maybe not this syntax but the same idea..... now I'm failing at code..