Deck of Cards Class

[TheNoobie]

I just put this together a little while ago just to see if I could. Let me know if you have any thoughts / comments and what you think of it. This is a class I made to create, shuffle and deal a real deck of cards.

Took me a little over an hour and it's definitely not done (still haven't even put in the deal function yet) but thought I would post it anyway since I am about to go to sleep.

Cards.h

#include <string>
#include <iostream>
#include <cstdlib>
#include <ctime>
#define EVER ;;

using namespace std;

class Cards
{
public:
	Cards() { }
	~Cards() { }
	void SetCardValues(int, int, int);
	void GetCardValues();
private:
	int Suit;
	int Value;
	int Index;
};

class DeckOfCards : public Cards
{
public:
	DeckOfCards();
	~DeckOfCards() { }
	void SetDeckValues(int, int, int, int);
	void GetDeckValues(int);
	void Shuffle();
	void Deal();
	Cards pCard[52];
};

Cards.cpp

#include "Cards.h"

void Cards::SetCardValues(int pSuit, int pValue, int pIndex)
{
	Suit = pSuit;
	Value = pValue;
	Index = pIndex;
}

void Cards::GetCardValues()
{
	cout << "Suit: " << Suit << endl;
	cout << "Value: " << Value << endl;
	cout << "Index: " << Index << endl;
}

void DeckOfCards::SetDeckValues(int ArrayNum, int pSuit, int pValue, int pIndex)
{
	pCard[ArrayNum].SetCardValues(pSuit, pValue, pIndex);
}

void DeckOfCards::GetDeckValues(int ArrayNum)
{
	pCard[ArrayNum].GetCardValues();
}

void DeckOfCards::Shuffle()
{
	srand( time(NULL) );

	for (int i = 1; i <= 250; i++)
	{
		for (int j = 0; j < 52; j++)
		{
			int n = rand() % 52;
			Cards tempCard = pCard[j];
			pCard[j] = pCard[n];
			pCard[n] = tempCard;
		}
	}
}

			

DeckOfCards::DeckOfCards()
{
	int value = 0;
	int CardNum = 1;

	for (int suit = 1; suit <= 4; suit++)
	{
		for (int card = 1; card <= 13; card++)
		{
			SetDeckValues(CardNum, suit, card, value);
			CardNum++;
			value++;
		}
	}
}

Proof of Concept

#include <iostream>
#include "Cards.h"

int main()
{
	int myChoice;
	char Shuffle;
	DeckOfCards DeckOne;

	for (EVER)
	{
		cout << "Should we shuffle? [Y]es or [N]o: ";
		cin >> Shuffle;

		switch (tolower( Shuffle ) )
		{
		case 'y':
			DeckOne.Shuffle();
			break;
		default:
			break;
		}

		cout << "Which card: ";
		cin >> myChoice;

		if (myChoice == 0)
			break;
	
		DeckOne.GetDeckValues(myChoice);
	}
	return 0;
}

[Athar]

Hm... some thoughts:
1. Using using namespace is a bad idea, as it injects the namespace in all translation units that include that header directly or indirectly, even if that isn't desired.
2. Cards:
2.1 The Cards class probably should be called Card.
2.2 Is it really necessary to change the card type later? If so, Suit and Value should be public - if not, you can move initialization to the constructor and remove SetCardValues. Then you'll need getters too and what looks like a getter right now should probably be called something along the lines of PrintCardValues, since that's what it does.
2.3 What's the purpose of "Index" in the card class?
2.4 Face and Suit should be enums. It might not be obvious to most people that 3 somehow stands for "Spades", as an example.

3.1 Likewise, the SetDeckValues and GetDeckValues don't seem particularly useful and their names aren't obvious. If necessary, you could provide a GetCard function that provides access to any card in the deck via index.
3.2. pCards should be a vector, as the number of cards in the deck will be variable as soon as you can draw cards.
3.3 As for the Shuffle function:
3.3.1 srand should be called once at the beginning of the program.
3.3.2 It's not necessary to repeat the shuffling for 250 times. Once is entirely enough. Note that this is exactly what random_shuffle does - so the entire function can be condensed to random_shuffle(pCards.begin(),pCards.end());
3.4 In the constructor, you should use push_back to add the cards. This will remove the need for SetDeckValues. There's an off-by-one error by the way - CardNum starts at 1 and is used as an index later.

4. Variables should be as local as possible. In particular, the declarations of myChoice and Shuffle should be moved to where they're used for the first time.

I think that's about it.

[Computergeek01]

I'm just throwing this out there but instead of relying on the std namespace, have you ever considered typing your code as you would normally then doing a find and replace before you compile it? I think this would be a nice combo of lazy and functional.

Shouldn't "index" be a static class member?

I actually did something like this not to long ago myself. I also had one class for cards and another for the deck but mine was slightly more complicated (I'm kind of an MTG nerd) and I wanted the classes to be usable for any game.

When you actually go to use this you'll want data members to keep track of where each card is for example; is it in someones hand, the deck or the discard pile? These are either individual bools (easy) or a single enumerated status counter (slightley more complicated).

[TheNoobie]

Thanks for all the advice, I probably should have said this in my original post but I was up all night...
-I try never to "use namespace" especially if I'm writing a library, but I was just testing out the different functions in console and it saved me a ton of time. I would definitely reformat my code when/if I decide to release it but in the meantime it's just much simpler this way.
-There really is no point of index, it was originally supposed to be an ID classification of each card but it will be removed since that's not needed (as of now.)
-The GetDeckValues/SetDeckValues aren't necessarily needed for this class but just added for my own ease while I'm just testing to see what values are being used in console.
-I planned on using enumerations to hold both the suits and face cards further down the line.

All in all..
Obviously much needs to be done to make this a fully functional library but like I said I only spent an hour on it and it's nowhere near complete. I really appreciate all the tips (I had actually never even heard of the random_shuffle function) and I will keep them all mind. Once I'm done this one I would love to look through yours to see the difference.

[Computergeek01]

I'd be happy to show you. I use SFML to display the graphics but that's easy enough to ignore if you don't know it. PM me when you're interested.

[TheNoobie]

I know SFML pretty well, so that won't be a problem. I am more interested in the actual class than the implementation anyway. I'll PM you when I finish mine.