"Iterator not dereferenceable"

Question

I love when code gives me headaches. Except this one. I am simply filling a vector with random doubles using an often used algorithm. The problem is, after it compiles, it gives me a "Iterator not dereferenceable" runtime error. Here is my code path:

-First I create an instance of a class.

 Random rm;

-Which runs this constructor

    Random::Random() // Constructor
{
    this->cAR();
    this->min = 0; // Default: Sets minimum to zero
    this->max = RAND_MAX; // Default: Sets maximum to RAND_MAX

    fillVect(); // Fills vector
    this->numCount = num.begin();
}

-cAR simply sets some values in below function

void Random::cAR()
{
    this->num.clear();
    this->num.reserve(250);
    this->place = 0;
}

-Now for the main problem (I think), I iterate thru the vector and fill it with random numbers generated

void Random::fillVect()
{
    srand(time(NULL));
    vector<double>::iterator counter;
    for (counter = this->num.begin(); counter <= num.end(); counter++)
    {
        *counter = (((double) rand() / (double) RAND_MAX) * (this->max - this->min)) + this->min;
    }

    shuffle();
}

-That should fill the vector (which is a "double" type vector, btw) with random numbers. Then, I call the next number in the vector (which should be the 0th index at the beginning) with this statement

double r = rm.nextDbl();

-Which is defined in this function

double Random::nextDbl()
{
    double temp = *numCount;
    return temp;
    numCount++;
    place++;
}

In my head, this is a correct code path. Now, I've only been coding for 2 years, and only 1 year as a student at an actual college, so my question would be, why is it giving me this runtime error? I appreciate any help beforehand.

Answer 1

You are dereferencing the end() iterator, which is invalid:

 for (counter = this->num.begin(); counter <= num.end(); counter++)

This should be:

for (counter = this->num.begin(); counter != num.end(); ++counter)

Answer 2

You have two problems in your code. First: you are dereferencing the end() iterator in your loop

for (counter = this->num.begin(); counter <= num.end(); counter++)
//                                        ^^ should be less than

Second: your num vector is empty so once you fix the first problem you will just end up with no numbers:

this->num.clear();       // num is now empty
this->num.reserve(250);  // num is still empty

I think you meant to call resize(250) instead of reserve(250). The latter just makes sure the vector has enough capacity... if it was already empty it will stay empty.

Also, two suggestions:

1. Consider using std::generate as a substitute for your loop. What you are doing is exactly generating values for num, so this algorithm will make it explicit to your readers.

2. Your approach to picking a random number is not uniform:

   (((double) rand() / (double) RAND_MAX) * (this->max - this->min)) + this->min;

That will not produce a uniform distribution, even if rand() itself produced a uniform distribution... and this will pretty much never give you this->max as a value. A much better solution, would be:

int nextRandom() {
    int range = this->max - this->min + 1;
    int max_rand = RAND_MAX / range * range; // largest multiple of range under RAND_MAX
    for (;;) {
        int r = rand();
        if (r < max_rand) {
            return (r % range) + this->min;
        }
    }
}