C++ Console Application1.exe has triggered a breakpoint

Question

when I try to set

cub.SetArray(cube);

I get an error

Console Application1.exe has triggered a breakpoint

What I'm doing wrong? When I try to debug cub -> cubesarray I get size -842150451. I don't understand why.Here's my all code

class Cube{
public:
    static const int Change_ARRAY = 5;

private:
    string color;
    int size;
    int *walls;
    int n; // current size of array
    int maximumsize; // maximum size of array
    void Increase(int many);
public:
    Cube(int maximumsize = 0);
    ~Cube();
    void SetWalls(int wall);
    void SetColor(string color);
    void SetSize(int size);

    string GetColor(){return color;}
    int GetWalls(int i){return walls[i];}
    int GetSize(){return size;}

    int GetN(){return n;}
};

Cube::Cube(int maximumsize):n(0), maximumsize(maximumsize), size(size), walls(NULL){
    if(maximumsize > 0){
        walls = new int[maximumsize];
    }
}

Cube::~Cube(){
    if(walls){
        delete [] walls;
    }
}

void Cube::Increase(int many){
    if(many > maximumsize){
        int *newest = new int[many];
        for(int i=0; i<n; i++)
            newest[i] = walls[i];
        delete [] walls;
        walls = newest;
        maximumsize = many;
    }else if( many < maximumsize){
        int *newest = new int[many];
        for(int i=0; i<many; i++)
            newest[i] = walls[i];
        delete [] walls;
        walls = newest;
        n = maximumsize = many;
    }
}

void Cube::SetWalls(int wall){
    if(n == maximumsize) Increase(n + Change_ARRAY);
    walls[n] = wall;
    n++;
}

void Cube::SetColor(string color){
    this->color = color;
}

void Cube::SetSize(int size){
    this->size = size;
}

class CubesArray{
public:
    static const int Change_Array = 5;
private:
    Cube *cubesarray;
    int currentsize; // current size of array
    int maxsize; // maximumsize
    void Change (int kk);
public:
    CubesArray(int maxsize = 1);
    ~CubesArray();

    void SetArray(Cube c);
    Cube GetArray(int ind){return cubesarray[ind];}
    int GetCsize(){return currentsize;}
};

CubesArray::CubesArray(int maxsize):cubesarray(NULL), currentsize(0), maxsize(maxsize){
    if(maxsize > 0){
        cubesarray = new Cube[maxsize];
    }
}

CubesArray::~CubesArray(){
    if(cubesarray){
        delete [] cubesarray;
    }
}

void CubesArray::Change(int kk){
    if(kk > maxsize){
        Cube *newarr = new Cube[kk];
        for(int i=0; i<currentsize; i++)
            newarr[i] = cubesarray[i];
        delete [] cubesarray;
        cubesarray = newarr;
        maxsize = kk;
    }if(kk < maxsize){
        Cube *newarr = new Cube[kk];
        for(int i=0; i<kk; i++)
            newarr[i] = cubesarray[i];
        delete [] cubesarray;
        cubesarray = newarr;
        currentsize = maxsize = kk;
    }
}

void CubesArray::SetArray(Cube cub){
    if(currentsize = maxsize) Change(currentsize + Change_Array);
    cubesarray[currentsize] = cub;
    currentsize++;
}

void Read(CubesArray & cub);

int main(){
    CubesArray cub;

    Read(cub);

    system("pause");
    return 0;
}

void Read(CubesArray & cub){
    string color;
    int size;
    int i=0;
    Cube cube;
    ifstream fd(Data);
    while(!fd.eof()){
        fd >> color >> size;
        cube.SetSize(size);
        cube.SetColor(color);
        cout << cube.GetColor() << " " << cube.GetSize() << " ";
        while(fd.peek() != '\n' && !fd.eof()){
            int w;
            fd >> w;
            cube.SetWalls(w);
            cout << cube.GetWalls(i) << " ";
            cub.SetArray(cube); // when I set cube to cub I get this error!!!
            i++;
        }
        cout << endl;
        fd.ignore();
    }
}

Just fyi, `std::vector walls;` in `Cube`, and `std::vector` would make an *significant* amount of this code disappear. And `size(size)` in your initializer list is wrong. I'm thinking `size(0)`? — WhozCraig, Mar 02 '14 at 08:44
I know, but I need to find out what's wrong with dynamic arrays — user3369351, Mar 02 '14 at 08:46
Start with the size() problem I mentioned, then work your way to the answers below (which may already include that). — WhozCraig, Mar 02 '14 at 08:49
Can you explain the specific purposes of `size` and `n` in class `Cube` and why you need *both* ? — WhozCraig, Mar 02 '14 at 08:57
No, size(size) in initializer is ok (I try to change to size(0) but still I got the same problem). I'm thinking that somehow array can't increase? — user3369351, Mar 02 '14 at 09:00
@user3369351 - I can make you Cube class go nuts with a 2 line main() program. The reason is that it violates the rule of three. http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — PaulMcKenzie, Mar 02 '14 at 09:02
"size(size) in initializer is ok " - No, `size(size)` in the initializer is **not ok**. Its is indeterminate until constructed, and you're constructing it with **itself**, thereby constructing it with an indeterminate value. There is no `size` parameter in the constructor arg-list. Whether it fixes your problem (and it won't) is not relevant. it is *undefined behavior* as-written and needs to be fixed *regardless*. — WhozCraig, Mar 02 '14 at 09:05

barak manos · Accepted Answer · 2014-03-02T09:17:35.183

7

Change:

if(currentsize = maxsize)

To:

if(currentsize == maxsize)

In addition, here is your real problem:

You have no copy-constructor in class Cube, so the walls array is not properly copied whenever you send a Cube instance by value, e.g., cub.SetArray(cube).

You must define it as follows:

Cube::Cube(const Cube& cube):n(cube.n),maximumsize(cube.maximumsize),size(cube.size),wall(NULL)
{
    if (maximumsize > 0)
    {
        walls = new int[maximumsize];
        for (int i=0; i<maximumsize; i++)
            wall[i] = cube.wall[i];
    }
}

And you have no assignment-operator in class Cube, so the walls array is not properly copied whenever you assign one Cube instance into another, e.g., cubesarray[currentsize] = cub.

You must define it as follows:

Cube& Cube::operator=(const Cube& cube)
{
    n = cube.n;
    maximumsize = cube.maximumsize;
    size = cube.size;
    wall = NULL;
    if (maximumsize > 0)
    {
        walls = new int[maximumsize];
        for (int i=0; i<maximumsize; i++)
            wall[i] = cube.wall[i];
    }
    return *this;
}

BTW, in the copy-constructor, you can simply call the assignment-operator (remove coding redundancy):

Cube::Cube(const Cube& cube)
{
    if (this != &cube)
        *this = cube;
}

edited Mar 02 '14 at 09:17

answered Mar 02 '14 at 08:46

barak manos

29,648
10
62
114

Thanks, but still I get Console Application1.exe has triggered a breakpoint – user3369351 Mar 02 '14 at 08:49
@barakmanos deally you want the exact *opposite* to be done. I.e. use a by-value assignment operator parameter and utilize the [copy/swap idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) for multiple reasons, among them exception protection. – WhozCraig Mar 02 '14 at 09:21
@WhozCraig: Thanks. Can you please elaborate on the potential flaws in the above copy-constructor/assignment-operator scheme? I've always used this scheme, so I would like to know what is conceptually (or practically) wrong with it. Thanks again... – barak manos Mar 02 '14 at 09:31
@barakmanos It mandates the object must undergo default-construction, *then* assignment. You want to avoid the default construction and instead utilize copy-constructor, then swapping instead. there is little downside to this and enormous upside. Exception safety is near the top of the list for up-side, btw. The link I provided does far better justice in describing it than i ever could in a simple comment post. – WhozCraig Mar 02 '14 at 09:49
@WhozCraig, thanks for the demo :) ... First of all, I still don't see the downside in my scheme. Second, I see a couple of downsides in the scheme you're proposing: 1. Passing argument by-value to the assignment operator (which means an additional call to the copy-constructor and an additional object **in the stack**). 2. Dependency on STL (or `std`, whichever way you call it). – barak manos Mar 02 '14 at 10:39
@barakmanos its not an *additional* call to the copy constructor. Its the *only* call to the copy constructor. Copy-sementatics in *one* place: the copy ctor. Your implementing it in once place, the assignment operator, which then requires *default* construction *before* copy-assignment, and a potential exception point-of-failure. Via copy/swap assignment uses the copy-ctor to create **the** copy as the value param, then swaps guts with that object, so on its destruction he cleans out your old entrails, and you take over his, which he successfully copied. – WhozCraig Mar 02 '14 at 11:31
...And `std::swap` isn't mandatory; its a convenience. If you don't like it, write your own. Every scheme has upshots and down shots. It isn't a mistake this is *the* scheme used for exception safety when dealing with dynamic membered objects. – WhozCraig Mar 02 '14 at 11:32
@ WhozCraig: OK, gotcha (didn't say it was a mistake, BTW, only wondered about the advantages/disadvantages in comparison with my scheme suggested above)... Thanks – barak manos Mar 02 '14 at 11:45

score 1 · Answer 2 · edited May 23 '17 at 12:22

Your Cube class violates the rule of three. Look here:

   void CubesArray::SetArray(Cube cub){  // calls copy constructor

That call creates a copy of your Cube class. Your Cube class is not safely copyable. Please see this and scroll down to the Managing Resources section: What is The Rule of Three?

You should pass Cube by reference or const reference, not by value. Doing so may correct the error you're having now, but still, your class is faulty.

C++ Console Application1.exe has triggered a breakpoint

2 Answers2

Linked