String method crashes program...

Question

Alright so i have two identical string methods...

string CreateCust() { 
    string nameArray[] ={"Tom","Timo","Sally","Kelly","Bob","Thomas","Samantha","Maria"};
    int d = rand() % (8 - 1 + 1) + 1;   
    string e =  nameArray[d];
    return e;
   }      

string CreateFood()  {
     string nameArray[] = {"spagetti", "ChickenSoup", "Menudo"};
     int d = rand() % (3 - 1 + 1) + 1;   
     string f =  nameArray[d];
     return f;
} 

however no matter what i do it the guts of CreateFood it will always crash. i created a test chassis for it and it always fails at the cMeal = CreateFood();

        Customer Cnow;        
        cout << "test1" << endl;
        cMeal = Cnow.CreateFood();
        cout << "test1" << endl;
        cCustomer = Cnow.CreateCust();
        cout << "test1" << endl;

i even switched CreateCust with CreateFood and it still fails at the CreateFood Function...

NOTE: if i make createFood a int method it does work...

Also guys even if i changed CreateFood to just COUT a message and nothing more it still crashed...

Answer 1

Take out the + 1 on both of them, you access arrays starting from 0:

int d = rand() % (8 - 1 + 1);  // 0-7, not 1-8
int d = rand() % (3 - 1 + 1);  // 0-2, not 1-3

Otherwise you're accessing a non-existent element, and this is undefined behavior. (That means it could appear to work, like in CreateCust, crash like in CreateFood, do nothing, or do anything.)

---

I'm not sure what the purpose of subtracting 1 then adding 1 is. In any case, now is the perfect time to learn: Don't Repeat Yourself. Even if you do something just twice, make a function out of it, it'll be less cryptic and more concise:

int random(int min, int max)
{
    return rand() % ((b - a) + 1) + a;
}

This is a simple function that returns a random number between a and b, inclusive. (Means it can include both a and b in the results.) Now your code reads:

// I'll leave CreateCust up to you

string CreateFood(void)
{
     string nameArray[] = {"spagetti", "ChickenSoup", "Menudo"};

     int d = random(0, 2); // either 0, 1, or 2, randomly   
     string f =  nameArray[d];
     return f;
} 

And you'll see even just one function makes it much easier to read; your goal is to make your code easy to read by humans. Also, this is much more concise:

string CreateFood(void)
{
     string nameArray[] = {"spagetti", "ChickenSoup", "Menudo"};

     return nameArray[random(0, 2)];
} 

Another bad thing to do is hardcode magic numbers into your program. For example, why 3 or 8? It can be deduced those are array sizes, but that doesn't stand on its own. What you might want is something like:

string CreateFood(void)
{
     const size_t ArraySize = 3; // 3 elements, 0-2
     string nameArray[ArraySize] = {"spagetti", "ChickenSoup", "Menudo"};
                    // ^ Ensure it matches

     return nameArray[random(0, ArraySize - 1)];
} 

Now the range for the number number makes sense up front.

---

The rest may be a bit advanced (which you won't understand until you get to templates), but shows how we might go on:

template <typename T, size_t N>
char (&countof_detail(T (&)[N]))[N];

#define countof(pX) sizeof(countof_detail(pX))

This nifty tool will give you the number of elements in an array. The code might turn into this

string CreateFood(void)
{
     string nameArray[] = {"spagetti", "ChickenSoup", "Menudo"};
                    // ^ no explicit size

     return nameArray[random(0, countof(nameArray) - 1)];
} 

We got rid of any numbers altogether, you can just manipulate the array as you please. Lastly, we're repeating ourselves again: getting a random element from an array. We should make a function for that:

template <typename T, size_t N>
T& random_element(T (&pArray)[N])
{
    return pArray[random(0, N - 1)];
}

This returns a random element from any array. Your function would then simply be:

string CreateFood(void)
{
     string nameArray[] = {"spagetti", "ChickenSoup", "Menudo"};

     return random_element(nameArray);
} 

Note in this refactoring (refactoring is taking code and factoring it into new, simpler parts) it reads much better: To get a food, we have an array of foods and we pick one at random.

Keep this kind of stuff in mind while you work, and as you learn C++ you can make better code. Anytime you repeat a task that isn't trivial, make it a function. Suddenly that task is trivial, because you don't care about how the function works (that's in the function), just what the function does (that's the function name).

Answer 2

Look here:

int d = rand() % (8 - 1 + 1) + 1;   

This will return a number between 1 and 8 inclusive. What you need is this:

int d = rand() % 8;

Answer 3

You're going outside the bounds of the array. The array object begins at 0.

Answer 4

I don't understand why you have

int d = rand() % (8 - 1 + 1) + 1;    

Why not just use

int d = rand() % 8;

Answer 5

The crash is happening because you are accessing an invalid index. This is because array indexes start from 0 and not 1, so you don't want to add a 1 to the rvalue of the modulus operator.

Here is a neat trick that you can use to make your code a little more maintainable:

template <class T>
T getRandElem( const T[] arr )
{
    return arr[ rand() % ( sizeof(arr) / sizeof((arr)[0]) ) ];
}

string CreateCust(){ 
    static string nameArray[] = {"Tom","Timo","Sally","Kelly","Bob","Thomas","Samantha","Maria"};
    return getRandElem<string>( nameArray ); 
}

string CreateFood(){
     static string nameArray[] = {"spagetti", "ChickenSoup", "Menudo"};
     return getRandElem<string>( nameArray );
} 

Answer 6

I think modern c++ compilers will still let you use an old C trick for static arrays:

string CreateFood()
{
     char* nameArray = {"spagetti", "ChickenSoup", "Menudo"};
     // note the trick to get the compiler to count the array elements for you:
     int d = rand() % (sizeof(nameArray) / sizeof(nameArray[0]));   
     return std::string( nameArray[d] );
}