Need help with objects in Liar's Dice

[Robo]

I'm new to classes and objects, and I am working on a Liar's Dice game for practice. I have come across a few issues.

I included a method, roll() and each instance rolls the same combination of random dice. I added sleep(1) to the method as a temporary fix, but it takes so long to roll.

Unless I use pointers the compiler gives me trouble about memory allocation. Is this the appropriate technique?

Also, I want to add an array that contains a count of how many of each die the computer has in its hand so that it can make some judgment when wagering. I made a method count(), but it only returns zeros.

Now, the program only displays each of the two players hands and the diceCount of the second hand (which are all zeros)

#include <iostream>
#include <string>
#include <time.h>
#include <cstdlib>
#include <unistd.h>
using namespace std;

//Player Class
class Player
{
public:

//constructor
Player(string theName, int dice=4);

//attributes
string name;
int diceLeft;
int hand[];
int diceCount[];

//methods
void roll();
void show();
void count();

};

//Player constructor definition
Player::Player(string theName, int dice)
{ name = theName;
diceLeft = dice;
}

//Player method definitions

void Player::roll()
{ srand(time(NULL));
for (int i = 0; i < diceLeft; ++i)
{ hand[i] = 1 + rand() % (6 - 1 + 1);
sleep(1); //SLOWS THE PROGRAM DOWN!
}
}

void Player::show()
{ for (int i = 0; i < diceLeft; ++i)
{ cout << hand[i] << endl;
}
}

void Player::count()
{ diceCount[6];
for (int i = 0; i < 6; ++i)
{ diceCount[i] = 0;
}

for (int n = 0; n < diceLeft; ++n)
{ switch (hand[n])
{ case 1:
diceCount[0]++;
break;

case 2:
diceCount[1]++;
break;

case 3:
diceCount[2]++;
break;

case 4:
diceCount[3]++;
break;

case 5:
diceCount[4]++;
break;

case 6:
diceCount[5]++;
break;
}
}
}


//MAIN FUNCTION
int main()
{

//Players
Player *p1 = new Player("Herald");
Player *p2 = new Player("Baloo");


//Everyone Rolls();
cout << "\nRolling...\n\n";
p1->roll();
p2->roll();

//Shows player own dice.
cout << "You rolled\n";
p1->show();
cout << endl;

p2->show();
cout << endl;
p2->count();

for (int i = 0; i<6; ++i)
{ cout << p2->diceCount[i] << endl;
}

delete p1;
delete p2;
return 0;
}
//END MAIN FUNCTION

[Faldrax]

Ok, I'll see if I can answer your questions:-)

The problem with the roll() method is the use if srand(time(NULL)); as the first line.
This initialises the 'random' number generator based on the current time (in seconds).
As the two calls to roll() are consecutive, the time is (almost always) the same (to the nearest second), so the random number generator is initialised to the same point both times, and so gives the same set of results.
Simple solution is to move the srand(time(NULL)); to main before you roll the first set of dice.

You do not say what problems the compiler reports on memory allocation, but my compiler (VS 2005) does not let me declare the arrays hand and dicecount without specifying the number of elements.
I modified the declaration of the class to add array bounds, and also to make some of the attributes private rather than public - idealy attributes should all be private and accessed only through appropriate public methods. This allows you to subsequently change the internal workings of the class without modifying the code that uses the class.
Anyway, using pointers and new / delete is perfectly valid - I've changed it to use objects directly as an example of how it would differ.

Lastly looking at the count() method I am not sure what is failing - I suspect it is the line diceCount[6] that is causing the problem, but as I could not compile your version (due to the hand[] problem above) I can't be sure. Looking at the code I did spot a more compact way of counting the dice.
hand[n] holds the dice roll for dice n (eg 3) so you can actulay use it to index the diceCount array, diceCount[hand[n]-1] is the count for the roll, so diceCount[hand[n]-1]++ does what your switch statement did (you need the -1 as the diceCount array runs from 0 to 5, rather than 1 to 6).

So with the changes, the code looks like

#include <iostream>
#include <string>
#include <time.h>
#include <cstdlib>
using namespace std;

const int DICE = 4;
//Player Class
class Player
{
private:
	string name;
	int diceLeft;
	int hand[DICE];
public:
	//constructor
	Player(string theName);
	int diceCount[6];

	//methods
	void roll();
	void show();
	void count();
};

//Player constructor definition
Player::Player(string theName)
	{ 
		name = theName;
		diceLeft = DICE;
	}

//Player method definitions

void Player::roll()
	{ 
		for (int i = 0; i < diceLeft; ++i)
		{ 
			hand[i] = 1 + rand()%6;
		}
	}

void Player::show()
	{ 
		for (int i = 0; i < diceLeft; ++i)
		{ 
			cout << hand[i] << endl;
		}
	}

void Player::count()
	{ 
		for (int i = 0; i < 6; ++i)
		{ 
			diceCount[i] = 0;
		}

		for (int n = 0; n < diceLeft; ++n)
		{ 
			diceCount[hand[n]-1]++;
		}
	}


//MAIN FUNCTION
int main()
{
		srand(time(NULL));
		//Players
		Player p1("Herald");
		Player p2("Baloo");

		//Everyone Rolls();
		cout << "\nRolling...\n\n";
		p1.roll();
		p2.roll();
		//Shows player own dice.
		cout << "You rolled\n";
		p1.show();
		cout << endl;

		p2.show();
		cout << endl;
		p2.count();

		for (int i = 0; i<6; ++i)
		{ 
			cout << p2.diceCount[i] << endl;
		}

		system("pause"); 
		return 0;
}

[Robo]

Thank you so much. This is very helpful. I was having issues declaring the array bounds within the class. Not knowing they had to be constants, I just didn't declare them. Your explanation is very clear. Hopefully, I won't have anymore trouble.