PRNG program failure. Cannot enter random amount of choices and will always answer with 2 from the PRNG

Question

So as the title describes I'm trying to learn PRNG but it hasn't gone too well thusfar. I am writing a program that takes an entered number of choices, rolls a dice and makes the choice for you. I do this using rand() and an array of strings with a length that is dependant on a variable read in before the declaration of the array.

The two problems showing up are that if I try to choose between for example 12 choices, the program will crash. As I read in the length of the array before I declare it I don't understand how this could happen. Also the PRNG will always return the number 2 which is not the desired function of a function that should return a random number between min and max. I will share my code below:

#include <iostream>
#include <cstdlib> // for rand() and srand()
#include <ctime> // for time()
#include <string>

using namespace std;

int callPRNG(int min, int max)
{
    srand(static_cast<unsigned int>(time(0))); // set initial seed value to system clock

    static const double fraction = 1.0 / (static_cast<double>(RAND_MAX) + 1.0);  // static used for efficiency, so we only calculate this value once
    // evenly distribute the random number across our range
    return static_cast<int>(rand() * fraction * (max - min + 1) + min);
}


int main()
{
    int max=1;
    int min=0;

    cout << "Please enter the amount of choices you want to choose from: ";
    cin >> max;
    string choices[max-1];
    int choiceMade=0;

    {
    int i=0;
        while(i<=max-1)
        {
            cout << "Please enter choice " << i+1 << ": ";
            cin >> choices[i];
            i++;
        }
        choiceMade=callPRNG(min, max-1);
        cout << "I have decided: " << choices[choiceMade-1];
    }

    return 0;
}

As extra information, I'm using code::blocks with C++11 enabled on a windows 10 OS. I hope someone can help me with this as I don't quite understand arrays and PRNG well enough to find the error by myself. Thanks :)

edit: It seems, as I can now use higher values for max without crashing the program, that the answer scales with the value of max.

Answer 1

The problem with crashing is because you're not allocating enough space for the array of choices (you need the size to be max, not max - 1). You're using a G++ extension to C++ — namely variable length arrays — which are part of standard C but not standard C++. You should use a vector of strings.

The non-random random behaviour is more a reflection on the quality of the rand() PRNG — it is often not very good. I was getting similar behaviour on macOS Sierra 10.12.2 with G++ 6.3.0. I worked around it by avoiding the use of floating point arithmetic and using modulus operations.

The use of choiceMade-1 was suspect too; given a choice of 0, it tries to access non-existent element -1 of the array of choices.

These changes yield the following code, which still has debug code in place:

#include <iostream>
#include <cstdlib>
#include <ctime>
#include <string>
#include <vector>

using namespace std;

int callPRNG(int min, int max)
{
    cout << __func__ << ": " << min << " .. " << max << "\n";

    unsigned int seed = static_cast < unsigned int > (time(0));
    cout << __func__ << ": " << seed << "\n";

    srand(seed);

    int range = max - min + 1;
    int max_valid = RAND_MAX - RAND_MAX % range;

    int rand_val;
    while ((rand_val = rand()) > max_valid)
        ;

    int rv = rand_val % range;
    cout << __func__ << ", rand_val = " << rand_val << ", ret_val = " << rv << "\n";

    return rv;
}

int main()
{
    int max = 1;
    int min = 0;

    cout << "Please enter the amount of choices you want to choose from: ";
    cin >> max;
    vector < string > choices(max);
    int choiceMade = 0;

    int i = 0;
    while (i <= max - 1)
    {
        cout << "Please enter choice " << i + 1 << ": ";
        cin >> choices[i];
        i++;
    }
    choiceMade = callPRNG(min, max - 1);
    cout << "I have decided: " << choices[choiceMade] << endl;

    return 0;
}

Sample outputs:

Please enter the amount of choices you want to choose from: 12
Please enter choice 1: a
Please enter choice 2: b
Please enter choice 3: c
Please enter choice 4: d
Please enter choice 5: e
Please enter choice 6: f
Please enter choice 7: g
Please enter choice 8: h
Please enter choice 9: i
Please enter choice 10: j
Please enter choice 11: k
Please enter choice 12: l
callPRNG: 0 .. 11
callPRNG: 1483236759
callPRNG, rand_val = 770034137, ret_val = 5
I have decided: f

Over a series of runs, with filtering out all except the 'I have decided' lines, I got outputs:

I have decided: g
I have decided: i
I have decided: d
I have decided: f
I have decided: a
I have decided: c
I have decided: l
I have decided: f
I have decided: a
I have decided: f
I have decided: h

Answer 2

Jonathan Leffler's answer was complete and correct. But I thought you might also be interested in a shorter program that uses <random>:

#include <vector>
#include <iostream>
#include <string>
#include <random>

size_t callRNG(size_t min, size_t max)
{
    std::random_device r{};
    std::uniform_int_distribution<size_t> ud{min,max};
    return ud(r);
}

int main()
{
    const auto min{0};
    std::vector<std::string> choices{};
    std::string line{};
    while(std::cout << "Please enter choice: ", std::getline(std::cin, line) && !line.empty()) {
        choices.push_back(line);
    }
    const auto max{choices.size() - 1};
    if(!choices.empty()) {
        std::cout << "I have decided: " << choices[callRNG(min, max)] << '\n';
    }
}