Deleting templated C++ 2-dimensional array

Question

Annoyance over C++'s requirement to pass a dimension in a 2-d array got me working on a templated Matrix class. I've been coding in C# for a bit, so I'm sure I'm a little rusty here.

Issue is, I get a heap exception as soon as I hit the destructor, which is trying to delete the 2-d array.

Any help gratefully accepted!

template <typename T>
class Matrix {
public:
    Matrix(int m, int n) : nRows(m), nCols(n) { 
        pMatrix = new T * [nRows]; 
        for (int i = 0; i < nCols; i++) {
            pMatrix[i] = new T[nCols];
        }
    }
    ~Matrix() { 
        if (pMatrix != NULL) { 
            for (int i = 0; i < nRows; i++) { delete[] pMatrix[i]; }
            delete[] pMatrix;
        }
    }
    T ** GetMatrix() const { return pMatrix; }
    T * Row(int i) const { return pMatrix[i]; }
    inline T Cell(int row, int col) const { return pMatrix[row][col]; }
    inline int GetNRows() const { return nRows; }
    inline int GetNCols() const { return nCols; }
private:
    int nRows, nCols;
    T ** pMatrix;
};

Answer 1

This is the bug:

for (int i = 0; i < nCols; i++) {
        pMatrix[i] = new T[nCols];
}

The loop should be until nRows, not nCols.

Other than that, let me tell you about something I did when I got tired of allocating 2-d arrays. I had to do a 3-d array. I used a map, that mapped from a coordinate - a struct holding x, y, z to the type I wanted.

I worked fast, and no need to allocate or deallocate. Assigning to a coordinate was simply done by

mymap[Coord(x, y, z)] = whatever...

Of course I needed to define the Coord struct and overload the < operator, but I found that way more comvenient than trying to allocate and deallocate a 3-d array.

Of course you will need to check if this scheme is fast enough for you. I used it to draw cells within a big cube using OpenGL and had no complaints at all.

Answer 2

Concerning the bug, @CodeChords_man explained it right. I have notes on implementation. I recommend to look through this wonderful FAQ post.

You should not use dynamic memory allocation unless you are 100% sure that

1. You really need it
2. You know how to implement it

I don't know of the first, and how the performance is crucial for you. But as for the second, you at least violated the rule of three. You class is very unsafe to use. If you copy it, the memory buffer will then be double-deleted.

You should not afraid to used STL containers, they are fast and optimized. At least the std::vector, it is as fast as the raw pointer in many scenarios. You can rewrite you class using std::vector as follows:

template <typename T>
class Matrix {
public:
  typedef std::vector<T> MatrixRow;
  typedef std::vector<MatrixRow> MatrixBody;

  Matrix(int m, int n) : nRows(m), nCols(n), _body(m, MatrixRow(n)) {}

  const MatrixBody& GetMatrix() const { return _body; }
  const MatrixRow& GetRow(int i) const { return _body[i]; }

  inline T Cell(int row, int col) const { return _body[row][col]; }
  inline int GetNRows() const { return nRows; }
  inline int GetNCols() const { return nCols; }
private:
  int nRows, nCols;
  MatrixBody _body;
};

Since this class is not using dynamic memory allocation, it is safe to copy and assign. You also don't need to explicitly store nRows and nCols in this case; you can use _body.size() and _body[0].size() instead.

Concerning underlying vector of vectors, it is dereferenced using the same [i][j] construction. It is easily iterated with begin() and end(). And if you absolutely need to use the raw pointer in some routine, you can always access it with &row[0].

The only possible difficulty is that you cannot easily convert MatrixBody to T**. But think it twice, maybe you don't really need to use T** at all.