Problem picking a random number within a range set

Question

I have this code:

cin >> command;
if (command[0] == 'R' or command[0] == 'r') {
    if (command[1] == 'o' and command[3] == 'l' and command[2] == 'l') {
        if (command[4] == '\0') {
            cout << rand()+ 1 << endl;
        } 
        else if (command[5] == '\0')
        {
            NumberName = command[4];
            cout << (rand()%NumberName)+ 1 << endl;
        }
        else if (command[6] == '\0')
        {
            NumberName = command[4] + command[5];
            cout << (rand()%(NumberName))+ 1 << endl;
        }
    }
}

Running it produces random numbers outside of the range.

command is an array with a limit of 30.

NumberName is just a normal int.

I enter roll10 in the console and it returns 90 then 34 then 51, you get the idea.

i probably should have included this but earlier in the script it asks the user for a input using cin >> command; — SomeRandomPerson 42, Apr 04 '20 at 05:32
`rand` is a pretty bad way of getting random numbers. I suggest using [uniform_int_distribution](https://en.cppreference.com/w/cpp/numeric/random/uniform_int_distribution); it's way more user-friendly and yields better numbers. If you are interested in why, see this question [why is the use of rand considered bad?](https://stackoverflow.com/questions/52869166/why-is-the-use-of-rand-considered-bad/52881465) — nick, Apr 04 '20 at 05:33
yes it is a char array but i dont really know how else to decompile the input — SomeRandomPerson 42, Apr 04 '20 at 05:45
You need to parse the string, ideally, you should put a separator between roll and 36, e.g. `roll:36` then you can split the string on the `:` and treat the left as a command and the right as a number, which you can convert with `atoi`. — nick, Apr 04 '20 at 05:48
thanks! im gonna try to find out how to actually split the string now — SomeRandomPerson 42, Apr 04 '20 at 06:09
@nick no need for `atoi()`, just put the `command` into a `std::istringstream` and then use `std::getline(':')` to extract `roll` and `operator>>` to extract `36` directly into the `int` — Remy Lebeau, Apr 04 '20 at 06:32

bitmask · Answer 1 · 2020-04-04T06:55:24.023

Please do not use rand() and in particular do not use % to shrink to a range because this produces a non-uniform distribution.

The C++ standard library specifically has a uniform_int_distribution type for this. Use this with mt19937 to generate pseudo-random numbers:

std::uniform_int_distribution<unsigned> dis(min, max);
std::mt19937 gen(seed);
auto const randomnumber = dis(gen);

In your example you probably get an unexpected range because the character '7' (for instance) does not equal the number 7. You might want to use character[4]-'0' as the maximum value.

When you enter roll10 this gets translated into the last branch of your conditional, which means you have the numerical value for '1' plus the numerical value for '0' which is 48+49 = 97. So x%97 + 1 allows for all values in [1..97] with an almost but not quite uniform distribution.

You probably are looking for something like this:

auto const max = static_cast<unsigned>(command[4]-'0')*10 + static_cast<unsigned>(command[5]-'0');
auto const randomnumber = std::uniform_int_distribution<unsigned>(1, max)(gen);

Problem picking a random number within a range set

1 Answers1