Declaration and assignation of dynamic 3d array

Question

I have made the next class for a 3d array. Declaring a variable as, for example

Grid3d n = Grid3d(2,2,2);
n(0,0,0) = 1;

works fine, but declaring it as

Grid3d n;
n = Grid3d(2,2,2);
n(0,0,0) = 1;

Gives me a segmentation fault, the problem seems to be the default constructor, but i don't know how to fix it, any clue?

#ifndef _GRID3D_
#define _GRID3D_

#include <iostream>
#include <cmath>
#include <cassert>  // assert()

using namespace std;

class Grid3d
{
private:
    int L;
    int M;
    int N;
    double *** G;

public:
    Grid3d(int,int,int);
    Grid3d();
    Grid3d(const Grid3d &);
    ~Grid3d();

    double & operator()(int,int,int);
};
#endif

//Constructor
Grid3d::Grid3d(int L,int M,int N)
    :L(L), M(M), N(N)
{
    int i,j,k;
    G = new double ** [L];
    for (i=0;i<L;i++){
        G[i] = new double * [M];
        for (j=0;j<M;j++){
            G[i][j] = new double [N];
            for (k=0;k<N;k++){
                G[i][j][k] = 0;
            }
        }
    }
}

//Constructor vacío
Grid3d::Grid3d()
    :L(0), M(0), N(0)
{
    G = NULL;
}

//Constructor copia
Grid3d::Grid3d(const Grid3d &A)
    :L(A.L), M(A.M), N(A.N)
{
    G = new double ** [L];
    int i,j,k;
    for (i=0;i<L;i++){
        G[i] = new double * [M];
        for (j=0;j<M;i++){
            G[i][j] = new double [N];
            for (k=0;k<N;k++){
                G[i][j][k] = A.G[i][j][k];
            }
        }
    }
}

//Destructor
Grid3d::~Grid3d()
{
    // Libera memoria
    for (int i=0;i<L;i++){
        for (int j=0;j<M;j++){
            delete [] G[i][j];
            G[i][j] = NULL;
        }
        delete [] G[i];
        G[i] = NULL;
    }
    delete G;
    G = NULL;
}

double& Grid3d::operator()(int i,int j,int k)
{
    assert(i >= 0 && i < L);
    assert(j >= 0 && j < M);
    assert(k >= 0 && k < N);
    return G[i][j][k];
}

Assignment operator

Grid3d Grid3d::operator = (const Grid3d &A)
{
    if (this == &A) {return *this;};
    if (G != NULL){
        // Libera memoria
        for (int i=0;i<L;i++){
            for (int j=0;j<M;j++){
                delete [] G[i][j];
                G[i][j] = NULL;
            }
            delete [] G[i];
            G[i] = NULL;
        }
        delete G;
        G = NULL;
    }

    L = A.L;
    M = A.M;
    N = A.N;
    G = new double ** [L];
    int i,j,k;
    for (i=0;i<L;i++){
        G[i] = new double * [M];
        for (j=0;j<M;i++){
            G[i][j] = new double [N];
            for (k=0;k<N;k++){
                G[i][j][k] = A.G[i][j][k];
            }
        }
    }
    return *this;
}

Why is your #endif statement halfway through the class definition? — ApproachingDarknessFish, Jan 20 '13 at 21:49
#endif is there because i have the declaration and implementation of the class in 2 files — user1995432, Jan 20 '13 at 21:52
You can store this with a `std::vector` with a size of L*M*N. — andre, Jan 20 '13 at 21:52
@juanchopanza 's answer is *the* answer, btw, (in case that wasn't obvious). There are much better ways to do what you're doing, but his comment is the immediate answer to your problem. — WhozCraig, Jan 20 '13 at 21:54
See this question: [What is the `rule of three`?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — Peter Wood, Jan 20 '13 at 21:57

score 2 · Answer 1 · answered Jan 20 '13 at 22:00

2

It's a trivial typo. I'll point out the line, and I'll let you spot where it's gone wrong:

for (j=0;j<M;i++)

That is, aside from the missing assignment operator, which makes the original constructed object in n = Grid3d(2,2,2); be freed, and thus you are accessing G[i][j][k] in operator() with NULL pointers.

answered Jan 20 '13 at 22:00

Mats Petersson

126,704
14
140
227

*totally* worth burning one my my final 3 daily votes for this. did *not* see that coming =P – WhozCraig Jan 20 '13 at 22:42

score 2 · Accepted Answer · answered Jan 20 '13 at 22:05

2

You have dynamically allocated memory, but you have not followed the rule of three. You are missing an assignment operator, so when you do this:

Grid3d n;
n = Grid3d(2,2,2); // both RHS temporary and n point to the same data.
n(0,0,0) = 1;      // Accessing deleted memory: undefined behaviour.

you will have two attempted de-allocations of the same memory, because you have n's pointer pointing to the same memory as the temporary used to assign to it in the second line. When the temporary dies, it de-allocated it's memory. When n dies, it tries to de-allocate the same memory. Furthermore, any access to that memory after the second line is undefined behaviour.

answered Jan 20 '13 at 22:05

juanchopanza

223,364
34
402
480

Since the destructor sets the inner pointers in G to NULL, it actually goes wrong in the access in operator() - I actually compiled and ran it, just to confirm. – Mats Petersson Jan 20 '13 at 22:26
@MatsPetersson Yeah, I added a comment in the code sample. But even without the access, there would eventually be a double delete. – juanchopanza Jan 20 '13 at 22:28
I added an assignment operator, but it not working either, i've edited the main post – user1995432 Jan 20 '13 at 22:40
@user1995432 sorry, I don't read functions that are more than five lines long. But you could use the [copy and swap idiom](http://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Copy-and-swap) and leverage the copy constructor. – juanchopanza Jan 20 '13 at 22:42

Declaration and assignation of dynamic 3d array

2 Answers2