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();
    }
}

Solution 1

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;
}

Solution 2

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.

Comments

[Admin]

Thanks, but still I get Console Application1.exe has triggered a breakpoint

[WhozCraig]

@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 for multiple reasons, among them exception protection.

[barak manos]

@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...

[WhozCraig]

@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.

[barak manos]

@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).

[WhozCraig]

@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]

...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.

[barak manos]

@ 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