Why does my srand return all outcomes?

Question

This is the beginning of a simple text game. In the game, you are supposed to go around finding dungeons and collecting artifacts. I use srand(time(0)) to do things such as find what stage to go to, attack, and what items you find, I haven' gotten far in the programming, but i have already encountered a problem. My rand() returns all outcomes. When i run the game (this is not the complete code, btw), it returns "You entered a dungeon!", "Oh No, an enemy has arrived!", and "you have found an artifact!

void mainScreen()
{
    srand(time(0));
    cout << "Health: \n";
    cout << health;
    cout << endl;
    _sleep(500);
    cout << "Inventory: \n";
    cout << inventory;
    cout << endl;
    _sleep(500);
    cout << "Gold: \n";
    cout << gold;
    cout << endl;
    _sleep(500);
    cout << "Artifacts: \n";
    cout << artifacts;
    cout << endl;
    _sleep(500);
    cout << "Rolling the dice of fate... \n";
    int diceRoll = 1 + (rand() % 10);
    if (diceRoll = 1, 2, 3, 4, 5, 6) {
        cout << "You entered a dungeon! \n";
    }
    if (diceRoll = 7, 8) {
        cout << "Oh No! An enemy has arrived! \n";
    }
    if (diceRoll = 9, 10) {
        cout << "You found an artifact! \n";
    }
}

What do you mean returns all outcomes? Also, == for comparison, || for logical or — Andrew Li, Sep 01 '16 at 00:22
`=` and `,` don't do what you think they do. Try consulting a reference on C++ instead of guessing... — M.M, Sep 01 '16 at 00:23
`diceRoll = 1, 2, 3, 4, 5, 6` is not how you check multiple values — NathanOliver, Sep 01 '16 at 00:24
[See here](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) — M.M, Sep 01 '16 at 00:27
In your case, you can test if a value is inside a range: If(diceRoll >= 1 && diceRoll <= 6) { ... } — PaulHK, Sep 01 '16 at 03:08

Remy Lebeau · Answer 1 · 2016-09-01T00:34:52.527

Your if statements do not do what you think they do.

First, you are using the = assignment operator when you should be using the == comparison operator instead.

Second, you are using the , operator, which evaluates both left and right expressions and then returns the result of the right expression.

So, this code:

if (diceRoll = 1, 2, 3, 4, 5, 6)
{
    ...
}
if (diceRoll = 7, 8)
{
    ...
}
if (diceRoll = 9, 10)
{
    ...
}

Is effectively doing the same as this, which is not what you want:

diceRoll = 1;
if (6)
{
    ...
}
diceRoll = 7;
if (8)
{
    ...
}
diceRoll = 9;
if (10)
{
    ...
}

You need to do this instead:

if ((diceRoll == 1) ||
    (diceRoll == 2) ||
    (diceRoll == 3) ||
    (diceRoll == 4) ||
    (diceRoll == 5) ||
    (diceRoll == 6))
{
    cout << "You entered a dungeon! \n";
}
else if ((diceRoll == 7) ||
        (diceRoll == 8))
{
    cout << "Oh No! An enemy has arrived! \n";
}
else
{
    cout << "You found an artifact! \n";
}

Which can be simplied using range comparisons:

if ((diceRoll >= 1) && (diceRoll <= 6))
{
    cout << "You entered a dungeon! \n";
}
else if ((diceRoll >= 7) && (diceRoll <= 8))
{
    cout << "Oh No! An enemy has arrived! \n";
}
else
{
    cout << "You found an artifact! \n";
}

Or replaced with a single switch statement:

switch (diceRoll)
{
    case 1:
    case 2:
    case 3:
    case 4:
    case 5:
    case 6:
    {
        cout << "You entered a dungeon! \n";
        break;
    }

    case 7:
    case 8:
    {
        cout << "Oh No! An enemy has arrived! \n";
        break;
    }

    case 9:
    case 10:
    {
        cout << "You found an artifact! \n";
        break;
    }
}

Also, on a side note, you should not be calling srand() every time mainScreen() is called (which I assume can be called multiple times during the program's lifetime). srand() should only be called one time, so you should call it in main() before calling mainScreen().

score 0 · Answer 2 · answered Sep 01 '16 at 00:49

This:

if (diceRoll = 1, 2, 3, 4, 5, 6) {
    cout << "You entered a dungeon! \n";
}
if (diceRoll = 7, 8) {
    cout << "Oh No! An enemy has arrived! \n";
}
if (diceRoll = 9, 10) {
    cout << "You found an artifact! \n";
}

Is completely wrong, you need to check each element seperately, like this:

if (diceRoll == 1 || diceRoll ==2 || diceRoll == 3 || diceRoll == 4 diceRoll == 5 || diceRoll == 6) {
    cout << "You entered a dungeon! \n";
}
if (diceRoll == 7|| diceRoll == 8) {
    cout << "Oh No! An enemy has arrived! \n";
}
if (diceRoll == 9 ||diceRoll == 10) {
    cout << "You found an artifact! \n";
}

To further simplify the first branch, you could do this:

if (diceRoll >= 1 || diceRoll <= 6) {
    cout << "You entered a dungeon! \n";
}

Why does my srand return all outcomes?

2 Answers2