Program failing after running as expected

[DCC1]

Hello, I am trying to write a simple random number generator. The goal of the program is to ask the user how many digits (how long a sequence) to be 'randomized'. The program works fine when the user enters a relatively low number, I.E. 4, but when the user enters 6 or 7 (or more), the program will execute, and perform as expected, but causes my compiler (Microsoft Visual Studio C++) to produce an 'Unhandled exception in random.exe: 0xC0000005: Access Violation' error as soon as the program ends. I am using Vista Premium 64 bit, and was warned upon installing my compiler that there existed 'compatibility issues', but I don't believe this is what is causing it. Here is the code (very short):

#include <iostream>
#include <stdlib.h>
#include <stdio.h>
#include <time.h>
using namespace std;

int main()
{
	int letter[4];
	int digits = 0;

	cout << "Enter the number of digits you want to randomize... Max is 10. \n\n";
	cin >> digits;

	if (digits > 10) {
		cout << "Too many digits. Max is 10\n";
		return main();
	}

	cout << "Randomizing " << digits << " digits...\n\n";

	srand (time(NULL)); //init seed based on runtime for random generator
	
	cout << "Random sequence is: \n\n";

	for (int n = 0; n < digits; n++) {
	letter[n] = rand()%9;
	cout << letter[n];
	}

	return 0;
}

I am not sure what is causing the exception error... Does it have something to do with the repeated calls to rand() because it is in a for loop? If so, why does this happen, and how would I go about working around this?

Thanks in advance for your time in reading/responding.

[firedraco]

Your letter[] is only a size of 4, but you accept up to 10 maximum...also, you don't need stdio.h or stdlib.h since you are using C++ iostream. I would also suggest changing time.h to ctime.

[S6k9GNh0]

Try handling the exception with try and catch.

[firedraco]

Oh...um also...

return main();

PLEASE don't use this! At least you aren't just calling main(), but it is still not a good idea. Use a loop instead.

[Disch]

int letter[4];
//...
letter[n] = rand()%9;

Here is your problem.

If digits is >= 4, then line 27 in your code is going out of bounds of the array, which is corrupting the stack and causing the crash.

A solution here is to either up the size of the 'letter' array to 10 (since that is your program's max digits), or to allocate the array dynamically with new[]:

int* letter;

// ... get digits from user
letter = new int[digits];

// ... do other stuff, then before you exit:

delete[] letter;
return 0;

Or use a vector:

#include <vector>

//...

vector<int> letter;

//...
for(int n = 0; n < digits; ++n)
{
  letter.push_back( rand()%9 );
  cout << letter[n];  // or cout << letter.back() -- whichever you prefer
}

Or... since you don't seem to be actually using letter, just remove it completely:

for(int n = 0; n < digits; ++n)
{
  cout << rand() % 9;
}

Another error / problem:

- line 17: NEVER CALL MAIN. NEVER NEVER NEVER. This kind of recursion is bad design even if main were a normal function. Instead, put the part you want to repeat in a loop:

int main()
{
    // <<insert your solution for 'letter' here>>
    int digits = 0;


    while(1)
    {
        cout << "Enter the number of digits you want to randomize... Max is 10. \n\n";
        cin >> digits;

        if (digits > 10)
            cout << "Too many digits. Max is 10\n";
        else
            break;    // exit loop if they input a valid number
    }

    // ... 

EDIT - bah I'm way too slow. 3 replies too late! haw

[DCC1]

Ahh how foolish of me (about the array overflow part). Thanks for the pointers... Promise I will spend more time rechecking my code before posting next time. I am now allocating the array dynamically, as suggested by Disch.

Also switched the header files - was not aware that one (ctime) type of header was preferred over another (time.h).

As far as the loop is concerned, I have replaced the recursive call to main() with a while loop, and instead of breaking (using the 'break' statement), I am using a 'goto' statement, and the program seems to behave as expected (re-asks for digits value instead of exiting).

I have read that using goto statements is generally not good practice unless absolutely necessary or in special situations, is this true, if so, why?

Thanks again.