D3r513g
D3r513g

Reputation: 33

Why does Program fail after conclusion

I am hoping someone can answer this and in effect teach me a little something. I have this simple little snippet of code and it works without error but then after the program ends Windows throws the following error

program.exe has stopped working A problem caused the program to stop working correctly. Windows will close the program and notify you if a solution is available.

With a button to Close Program

The code in question below asks a user how many players there will be, then based on the number of players creates an array of a size equal to the number of players. Then a for loop prints each character name to the screen. Here is the code

int main()
    {
        int numplay;
        cout<<"How many players will there be? ";
        cin>> numplay;
        cin.ignore();
        cin.get();
        string *players = new string[numplay - 1]; 
        for (int x = 1; x < numplay + 1; x++) {     
        string name;
        cout<<"What is Player "<< x <<"'s name? ";
        cin>> name;
        players[x - 1] = name;
        cin.ignore();
        }
        cin.clear();
        cin.sync();
        cin.get();
        for (int x = 0; x < numplay; x++) {
        cout<< players[x] <<"\n";
        }
        delete[] players;
    }

The thing is like I said the code compiles and runs fine it's just at the end Windows throws the error mentioned above with few details. The problem is alleviated if you remove the -1 from the array declaration. However then that creates an extra unused array element. I hope this question is coherent it is one born completely from curiosity since Windows didn't give many details.

Upvotes: 0

Views: 49

Answers (3)

Olaf Dietsche
Olaf Dietsche

Reputation: 74018

You allocate numplay - 1 array elements, e.g elements 0 ... numplay - 2

string *players = new string[numplay - 1];

But in the loop, you access elements from 0 to numplay - 1 inclusive, which is one element beyond the array

for (int x = 1; x < numplay + 1; x++) {
    ...
    players[x - 1] = name;
}

In the last iteration, you have x = numplay. With this, you access players[numplay - 1], which lies outside of the array's bounds.

This is also the reason, why doing

string *players = new string[numplay];

fixes your problem. Because now, the array elements go from 0 to numplay - 1, which fits with the accesses in the for loop.

Upvotes: 1

paddy
paddy

Reputation: 63461

You are accessing the array out of bounds. As you hinted at, removing the -1 from the array allocation makes it work.

string *players = new string[numplay - 1];   // Wrong

If the user enters 3, then you will only allocate an array with 2 elements. That number represents the number of elements, not the maximum index.

The correct code is:

string *players = new string[numplay];

I also suggest you use zero-based indexing for any loop that operates on the array. It's confusing to see a loop like the one that follows the above line. Do this instead:

for (int x = 0; x < numplay; x++) {     
    cout << "What is Player "<< x+1 <<"'s name? ";
    cin >> players[x];
    cin.ignore();
}

Upvotes: 1

Beta
Beta

Reputation: 99094

string *players = new string[numplay - 1]; 
for (int x = 1; x < numplay + 1; x++) {     
  ...
  players[x - 1] = name;
}
...
for (int x = 0; x < numplay; x++) {
  cout<< players[x] <<"\n";
  ...
}

You are modifying and using elements outside the bounds of the array. This causes undefined behavior.

Upvotes: 0

Related Questions