have a logic problem that i can't even see

i write a program that use random numbers to simulate a dice. The problem is that when i roll the rice both random generates are give me the same number even thought i use the time function as the seed.

Post the smallest and simplest compilable program that demonstrates the problem.

Code:

---

#include <iostream>
#include "Dice.h"
#include <ctime>
using namespace std;

/* Constructor that initializes data member */
Dice :: Dice ()
{
    dice1 = 0;
    dice2 = 0;
}//end constructor



/* Set content of dice1 data member */
void Dice :: setDice1 ()
{
    srand ( time (0));
    dice1 = 1 + rand ()% ( 6 - 1 + 1 );
}//end setDice



/* Set content of dice2 data member */
void Dice :: setDice2 ()
{
    srand ( time (0));
    dice2 = 1 + rand () % ( 6 - 1 + 1);
}//end setDice2


/* Gets content of dice1 data member */
int Dice :: getDice1 ()
{
    setDice1 ();
    return dice1;
}//end getDice1


/* Gets the content of dice2 data member */
int Dice :: getDice2 ()
{
    setDice2 ();
    return dice2;
}//end getDice2

---

Ah. Don't call srand at this point. Call it once, at the start of your global main function.

what do you think about that code in regards to indentation and logic

Your indentation is fine. As for logic: why do you call setDice1 in getDice1? If you are going to do that, then you only need getDice: you don't need any member variables.

the reason i did that is because i want each of my functions to solve one problem. if i did not call setDice in getDice when i call the getDice function in main it would only return 0 ( note: i would have to call setDice first and i think call it in getdice would fix that problem).

Quote:

---

Originally Posted by mdennis10

if i did not call setDice in getDice when i call the getDice function in main it would only return 0 ( note: i would have to call setDice first and i think call it in getdice would fix that problem).

---

Looking at the interface that your class provides, I would expect that I should call setDiceX, then I can access getDiceX as many times as I want to get the same value. Changing the value on me would come as an unpleasant surprise. On the other hand, if there was only getDice (which could be a non-member function), then I expect it to return a random die roll each time it is called.

By the way, I believe dice is plural. The singular form should be die.

i wanted to do this in OOD and that's why i used data members

Well, I'm telling you my reaction to the interface and its behaviour. You are of course free to ignore my feedback in the name of OOD, but OOD is a means to an end: just because you have member variables doesn't mean that you have a good OOD.

what exactly is soooo bad about it

The setDiceN member functions don't make sense when you are calling them from getDiceN. Are they private member functions? If so, then that's fine, but then you also don't need the member variables since they are always getting overwritten only.

i understand what you are saying but one of the things i took in consideration was that if the user call the getDice function without first calling the setDice function it would creat a problem, so to fix that i taught by eliminating the need for them to call it by calling it implicitly for them in getDice would be a good idea

Quote:

---

Originally Posted by mdennis10

one of the things i took in consideration was that if the user call the getDice function without first calling the setDice function it would creat a problem

---

So, instead of initialising the member variables to 0, "roll the dice" in the constructor. Renaming setDiceN to rollDiceN might also be a good idea.

ok thanks