-6

Here it is:

#include <iostream>    
#include <cstdlib> // for rand() and srand()
#include <ctime>
using namespace 
int main()
{
   //cout << "How many players?" << endl;
    int numplayers=1;
   //cin >> numplayers;

    int players[numplayers];
    int x=0,y=0;
    srand(time(0));
    x=(rand() % 6 + 1);
    y=(rand() % 6 + 1);
    players[1]=players[1]+x+y;
    cout << ("Your score is" + players[1]) << endl;
    cin >> numplayers;       
}

Ok My original problem was that this always crashed, now it prints "@"???

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055

4 Answers4

3

C++ arrays are 0 based.

players[1] is accessing a location outside the range of the array.

You will want: players[0].

Tim Beaudet
  • 791
  • 7
  • 16
  • While that would fix the problem, it isn't what you want. When you try to access the array for the first player, you want to access players[0]. For the second player, it would be players[1]. – Tim Beaudet Sep 21 '16 at 18:06
  • Are you seriously suggesting, that the fix is to adjust the index, instead of - say - stop the entire array-of-size-1 madness? – IInspectable Sep 21 '16 at 18:25
0

Definitely it crashes for the below line:

players[1]=players[1]+x+y;

Because size of players array is 1, so it has only index 0. And in above line, it tries to access index 1. Take a look at Buffer Overflow and array index out of bounds.

  • Try to define an array with constant size.
  • If you need a dynamic array, use linked list or vector.
nim_10
  • 477
  • 8
  • 21
  • 1
    Using extra space to avoid access overflow is not the best advice, you should be able to create an array of a desired size and use it properly by indexing from 0 to arraySize - 1 – Tim Beaudet Sep 21 '16 at 18:07
  • Sorry the code i gave in my post was outdated and has now been updated. I had already changed it to player[0], but thanks for being helpful. However now it just prints out a "@" instead of the message? – Colonbracket Sep 21 '16 at 18:09
  • 2
    *"Which causes Access Overflow for the program."* - No. It's undefined behavior. You cannot produce a definitive answer to a construct, that isn't defined. *"Always take some extra indices in case of array size to avoid access overflow."* - Now frankly, **that** was the reason to down vote on this proposed answer. You cannot possibly be serious about this. – IInspectable Sep 21 '16 at 18:28
  • I appreciate the mention of "taking extra spaces", I have updated the answer. Always used to take extra spaces, but after all it's really a bad idea, I should not have included that into my answer. – nim_10 Sep 21 '16 at 20:02
  • Still bogus, I mean, what the hell is an *"Access Overflow"*? A library granting too much access to an object, so that the client code overflows? And a statement that attributes verifiable and reproducible outcome to code that exhibits undefined behavior is just wrong, no matter how you slice it. – IInspectable Sep 22 '16 at 09:53
  • I have got your point and updated the answer. Also I found [this post](http://stackoverflow.com/questions/1239938/accessing-an-array-out-of-bounds-gives-no-error-why) very helpful. – nim_10 Sep 23 '16 at 17:00
0
cout << ("Your score is" + players[1]) << endl;

Here you're trying to use direct string concatenation, but you can't concatenate a string literal with an integer like that. You end up doing pointer arithmetic, which is definitely not what you intended.

You should instead use cout's built-in formatting:

cout << "Your score is" << players[1] << endl;

Your next problem is that players is not declared correctly; an array cannot have runtime bounds, and numplayers (despite having only the one initial value) is ultimately a "runtime" variable. You would be better off with a std::vector if the size of the array is going to change later; otherwise make numplayers a constexpr.

Your last problem is that, if the array declaration were valid, you'd be trying to access the second element of a one-element array! The first element is players[0], not players[1].

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
0

There are many things you're doing not good.

  • I believe you're using C++, there is no known way to me about creating arrays of dynamic size i.e. int players[numplayers]. In C++ either we can create array of fixed size i.e. int players[10] or use pointer to an array for dynamically allocated memory e.g. int* players = new int[numplayers]. This allocates an array of size numplayers and an int pointer named players is pointing to it. We can use this pointer as normal array e.g. printing 1st index of array is written as player[0] or another syntax is *(player + 0). Remember to delete this dynamically allocated memory at the end of program i.e. delete[] player.
  • Second thing is when you have allocated an array and are using its value for computation, always initialize it to 0 as newly allocated array contains garbage value and it will affect your computations. code for initializing it to zero may be like this:

Here is the loop:

for(int i = 0 ; i < numplayers ; i++){
    player[i]=0;
    //another syntax is : *(player + i) = 0;
}
  • C++ does not concat as you did in your std::cout statement. Make it like this: cout << "Your score is" << players[0] << endl;
  • In C++, arrays always start with index zero, so 1st index will be 0 in this case. So your program will work well if it is like this:

    int numplayers = 1;
    int* players = new int[numplayers];
    int x = 0, y = 0;
    srand(time(0));
    x = (rand() % 6 + 1);
    y = (rand() % 6 + 1);
    players[0] = 0;
    players[0] = players[0] + x + y;
    cout << "Your score is" << players[0] << endl;
    delete[] players;
    
    return 0;
    
Awais
  • 319
  • 3
  • 13