0

I have two classes, one called Handler and one called Dice. In my Handler class i have a private variable called Dice **dices and a public function called rollDices. And in my Dice class i have a function called toss that will randomize a number 1-6. The problem is that when the function rollDices is calling the function toss I get EXT_BAD_ACCESS in toss function. Does anyone know why, and have a solution for it?

My Handler.cpp:

void Handler::rollDices(){
    Dice **allDices = new Dice*[this->nrOfDices];
    this->dices = allDices;
    dices[nrOfDices]= new Dice(nrOfDices);
    int count =1;
    for (int i = 0; i < this->nrOfDices; i++)
    {
        allDices[i]->toss();
        cout << "Dice "<< count << ": " << allDices[i]->getValue() << endl;
        count ++;
    }
}

My Dice.cpp:

void Dice::toss(){
    this->value = rand()%this->nrOfSides+1; //Value is a private int in Dice class
}

If you need more code i can post it, just tell me!

pottsork
  • 33
  • 6
  • 1
    Not related to your problem, but "dice" is actually the plural of "die"; "dices" is incorrect. – Keith Thompson Mar 01 '16 at 21:00
  • haha sorry, not that good at english! – pottsork Mar 01 '16 at 21:09
  • The fact that "dice" is the plural of "dice" is one of those things that even native speakers of English can easily miss. Another fun fact: "opera" is the plural of "opus". – Keith Thompson Mar 01 '16 at 21:15
  • Ok, then i guess i'm not the only one who missed it! I will change dices to die in my code! Thanks :) – pottsork Mar 01 '16 at 21:18
  • *dices[nrOfDices]= new Dice(nrOfDices);* is confusing because where is dices declared and allocated? is it a global? is it a typo and you meant to type *this->dices[nrOfDices] = new Dice(nrOfDices);* (which would be an error since you reference past the end of the array)? – riqitang Mar 01 '16 at 21:49
  • Thanks for pointing out, i fixed the typo and if you look at my code now it should make a little more sense? – pottsork Mar 01 '16 at 21:52

3 Answers3

0
Dice **allDices = new Dice*[nrOfDices];

Allocates the top level pointer so now we have all of the rows in memory. When you add the columns

dices[nrOfDices]= new Dice(nrOfDices);

This does not add a new Dice to all of the rows. It adds a new Dice to one past the end of valid range of dices. What you need to do is use a loop and go through all of the rows and add a Dice to each one like

for (int i = 0; i < nrOfDices; i++)
    dices[i] = new Dice(nrOfDices);
NathanOliver
  • 171,901
  • 28
  • 288
  • 402
  • I understand what you say and everything seems logical, however, I get the same problem as before. I added dices[i] = new Dice(nrOfDices); to my loop but now i get error: EXC_ARITHMETIC – pottsork Mar 01 '16 at 21:12
  • @pottsork What is `allDices` and how is it initialized? – NathanOliver Mar 01 '16 at 21:16
  • Isn't allDices just a pointer to pointer array which points to class Dice that i allocate memory to with size of nrOfDices? Cause this is the first time in my code that i use allDices. – pottsork Mar 01 '16 at 21:21
0

You are only allocating a single Dice object at index nrOfDices (which is of bounds by the way), if you want to allocate all Dice objects you need:

void Handler::rollDices(){
    Dice **allDices = new Dice*[nrOfDices];
    this->dices = allDices;
    int count =1;
    for (int i = 0; i < this->nrOfDices; i++)
    {
        dices[i] = new Dice(nrOfDices);
        allDices[i]->toss();
        cout << "Dice "<< count << ": " << allDices[i]->getValue() << endl;
        count ++;
    }
}
0

How about using modern C++? Try something like this:

    void Handler::rollDice()
    {
        std::vector<Dice> allDice( nrOfDice );
        int count = 1;
        for( const auto & d : allDice )
        {
            d.toss();
            cout << "Die "<< count << ": " << d.getValue() << endl;
            ++count;
        }
    }
Rob K
  • 8,757
  • 2
  • 32
  • 36
  • Hmm, I have not learned about vectors yet and I feel that I should stick to the standard that I show above, sorry. – pottsork Mar 01 '16 at 21:35
  • So this is homework then? Cause what you show above is not standard, it's archaic, and should *never* be used in new production code. If you tried to do that code on my team, I'd make you rewrite it. – Rob K Mar 01 '16 at 21:38
  • Yes, homework. That's strange because this is what we are learning in school. I will tell my teacher to teach us modern C++ instead! Thanks for pointing this out! – pottsork Mar 01 '16 at 21:45
  • No, it's not really strange. Lots of courses have taught C++ as C with classes. And just because it shouldn't be used in production code doesn't mean you shouldn't understand it. A proper understanding of how memory management works is vital to being a good programmer. std::vector does all that sort of pointer management behind the scenes for you. If I were teaching a course, you'd do a lot of that sort of memory management. A programmer who understands memory will never leak it; a programmer who doesn't understand memory will always leak it. – Rob K Mar 01 '16 at 22:01
  • Ok i understand! I like you quote, that so true! Though i am the kind of programmer who will always leak it, hopefully i will get better! Started the course i C++ 6 weeks ago so my understanding of memory isn't that good, yet! – pottsork Mar 01 '16 at 22:09