0

How do I delete a dynamically created matrix? This is likely a duplicate, for which I apologize, but I really can't find a clear answer on here so far. I initialize a matrix as follows:

float ** createMatrix(unsigned int Nrows, unsigned int Ncols) {
    float ** result = NULL;

    if (Nrows != 0 && Ncols != 0) {
        // create the matrix on the heap
        result = new float * [Nrows];
        result[0] = new float [Nrows*Ncols]();

        // link the rows
        for (int i = 1; i < Nrows; i++) {
            result[i] = result[i-1] + Ncols;
        }
}

Now, I wish to create a function to delete it. Do I need two separate statements to delete M[0] and M, or just one for M? i.e. do I need:

void deleteMatrix(float **M){
    delete[] M[0];
    delete[] M;
}

OR SIMPLY:

void deleteMatrix(float **M){
    delete[] M;
}

Any help/explanation would be massively appreciated. Both versions "work" and don't show any errors to me in the console when deleteMatrix(M) is run, so I'm confused. Thanks!

qp212223
  • 19
  • 4
  • 2
    Possible duplicate of [Deleting a dynamically allocated 2D array](https://stackoverflow.com/questions/30720594/deleting-a-dynamically-allocated-2d-array) – wally Apr 04 '18 at 00:56
  • 2
    Your last two code blocks are the same – Tomáš Zahradníček Apr 04 '18 at 01:02
  • 1
    `result` is never returned. You have a memory leak if the second new[] throws an exception. Your function in one line, way better: `std::vector your_matrix(Nrows*Ncols);`. – O'Neil Apr 04 '18 at 01:04
  • @wally This is not the same type of allocation here: we only have two allocations. – O'Neil Apr 04 '18 at 01:06
  • 2
    [Link to what is often a better way to do this.](https://isocpp.org/wiki/faq/operator-overloading#matrix-subscript-op) – user4581301 Apr 04 '18 at 01:13
  • Sorry about them being the same, I copy pasted and forgot to change because I'm an idiot. I understand there may be better ways to do this, but it's for an assignment that uses this structure so I've gotta do it this way. Thanks. – qp212223 Apr 04 '18 at 05:11
  • Do what you have to do to pass the class. Anything else is a waste of your time. As a few other folk here have pointed out, two allocations requires two deletes, or option 1. You won't get an error if you neglect to delete an allocation until you run out of dynamic storage and can't allocate anymore. C++ doesn't track the kind of book-keeping needed to track lost allocations because of the "You only pay for what you use" philosophy. You can write your own manager to do this if you want.. – user4581301 Apr 04 '18 at 05:31

3 Answers3

0

These "2D array" questions come up constantly. I think I'll answer one.

Don't use arrays[]. Don't use new[] and delete[]. Just don't. Use std::vector<std::vector<int>> and let the miracle of C++ do all the newing and deleting for you. Or for something serious, use a well-designed open source library, like boost::matrix. C++ is way cool.

The following is a starter-kit. It can be improved, "privatized", and abstracted in lots of ways.

#include <vector>
using std::size_t;

template<class T>
struct Matrix {
    using matrix_type = std::vector<std::vector<T>>;
    matrix_type matrix;
    Matrix(size_t rows, size_t cols)
        : matrix(rows, matrix_type::value_type(cols)) 
    {}
};

int main() {
    size_t Nrows = 5u;
    size_t Ncols = 2u;
    Matrix<int> mx(Nrows, Ncols);

    auto& matrix = mx.matrix;  // Now use matrix[i][j] or whatever.

    // Here you can do anything with matrix that your could do with 
    // an array  or arrays ... and more. And it cleans up after iself.
 }
Jive Dadson
  • 16,680
  • 9
  • 52
  • 65
  • Don't write constructors like this. Just don't. `Matrix(std::size_t h, std::size_t w): matrix(h, matrix::value_type(w)) {}`. – bipll Apr 04 '18 at 06:27
  • @bipill - I first wrote it with size_t. I changed it because of how the OP defined Nrows. -- But for you... anything. – Jive Dadson Apr 04 '18 at 06:39
  • @bipill There. I fixed it so the OP will be totally confused. Bwahaha. – Jive Dadson Apr 04 '18 at 06:49
0

As many other's have stated every time you use new[] you have to have a matching [] delete. However as it currently stands with your functions and how they are declared/defined I believe you are running into an X/Y problem.

Here is why!

You propose to declare your delete function as such: Also you stated that both versions work and don't show any errors... well in your posted question both versions are exactly the same...

void deleteMatrix( float** M ) {
    // delete[] rows
    // delete[] cols
}

The problem that I'm seeing here is that as you pass in a pointer to a pointer of floats, the function does not know the dimensions of the matrix. It could be a 2x2, 2x3, 3x2, 3x3, MxN etc. Without knowing the dimensions of the matrix how are you able to write the for loop to traverse the inner or nested arrays? You would have to pass those dimensions into the delete function:

void deleteMatrix( float** M, int rowSize, int colSize ) {
    for ( int row = 0; row < rowSize; row++ ) {
        delete [] M[i];
    }
    delete [] M;
}

Here is an example similar to what you are trying to implement: thispointer.com

Outside of your actual problem; this tends to be more of a C approach or an outdated C++ approach. It ill advised to use raw pointers and new & delete freely. Using smart pointers is better. However for such a construct such as a matrix class, there are a plethora of freely usable libraries out there that have already defined such classes - interfaces to use and some of the most popular ones are:


Edit - User PaulMcKenzie cleared up a valid point that I have long forgotten since I've been mostly using vector<object> or vector<shared_ptr<object>>. It's been over 15 years since I've first learned about pointers - multi-dimensional arrays and I had forgotten about the concept of about contiguous arrays. The answer to a question that he posted to me in the comment section gives a clear explanation of what I've forgotten; found here. If the arrays are not contiguous in memory then yes it would be an X/Y problem without knowing their dimensions, but since they are; the sizes are not really needed to be known. And what you've already proposed should then work:

void deleteMatrix(float **M){
    delete[] M[0];
    delete[] M;
}

Edit - I was going back through some of my classes in my libraries and here is a template matrix class that I've written with any dimensional size matrix MxNx...ith It is very versatile in its ease and use. You can expand upon it if you want: I have not done any type checking or assertions but that can easily be added.

Using it is as simple as:

#include <iostream>
#include "Matrix.h"

int main() {
    Matrix<int, 2, 2> imat3x3( 1, 2, 3, 4, 5, 6, 7, 8, 9 );

    // calling elements() and using vector's [] operator
    for ( int i = 0; i < 9; i++ )
        std::cout << imat3x3.elements()[i] << ' ';            
    std::cout << '\n';

    // Using class's [] operator
    for ( int i = 0; i < 9; i++ )
        std::cout << imat3x3[i] << ' ';
    std::cout << '\n';

    // Using class's () operator
    for ( int i = 0; i < 9; i++ )
        std::cout << imat3x3(i) << ' ';
    std::cout << '\n';

    // Okay that was a 3x3 matrix of ints, lets do a 2x2x2 matrix of floats
    Matrix<float,2,2,2> fmat2x2x2( 0.1f, 0.2f, 0.3f, 0.4f, 0.5f, 0.6f, 0.7f, 0.8f );

    // now the operators
    for ( int i = 0; i < 8; i++ ) {
        std::cout << fmat2x2x2[i] << "f ";
    std::cout << '\n';

    for ( int i = 0; i < 8; i++ ) {
        std::cout << fmat2x2x2(i) << "f ";
    std::cout << '\n';          

    std::cout << "\nPress any key and enter to quit.\n";
    std::cin.get();
    return 0;
}

Matrix.h

#ifndef MATRIX_H
#define MATRIX_H

#include <vector>
#include <algorithm>
#include <numeric>

template<typename Type, size_t... Dims>
class Matrix {
public:
    static const size_t _numDims = sizeof...(Dims);

private:
    size_t _numElements;

    std::vector<Type>   _elements;
    std::vector<size_t> _strides;

public:
    Matrix() noexcept;

    template<typename... Args>
    Matrix( Args&&... args ) noexcept;

    const Type& operator[]( size_t idx );
    const Type operator[]( size_t idx ) const;

    const Type& operator() ( size_t idx );
    const Type operator() ( size_t idx ) const;

    size_t numElements() const {
        return _elements.size();
    }

    const std::vector<size_t>& strides() const {
        return _strides;
    }

    const std::vector<Type>& elements() const {
        return _elements;
    }
};    

#include "Matrix.inl"

#endif // !MATRIX_H

Matrix.inl

template<typename Type, size_t... Dims>
Matrix<Type, Dims...>::Matrix() noexcept :
_strides( { Dims... } ) {
    using std::begin;
    using std::end;
    auto mult = std::accumulate( begin( _strides ), end( strides ), 1, std::multiplies<>() );
    _numElements = mult;
    _elements.resize( _numElements );
}

template<typename Type, size_t... Dims>
template<typename... Args>
Matrix<Type, Dims...>::Matrix( Args&&... args ) noexcept :
_elements( { args... } ),
_strides( { Dims... } ) {
    _numElements = _elements.size();
}

template<typename Type, size_t... Dims>
const Type Matrix<Type, Dims...>::operator[]( size_t idx ) const {
    return _elements[idx];
}

template<typename Type, size_t... Dims>
const Type& Matrix<Type, Dims...>::operator[]( size_t idx ) {
    return _elements[idx];
}

template<typename Type, size_t... Dims>
const Type Matrix<Type, Dims...>::operator()( size_t idx ) const {
    return _elements[idx];
}

template<typename Type, size_t... Dims>
const Type& Matrix<Type, Dims...>::operator()( size_t idx ) {
    return _elements[idx];
}

Matrix.cpp - This cpp file is not necessary I only have it to easily compile it while debugging the class for basic compiler errors

#include "Matrix.h"

I did not demonstrate the use of the numElements() or stride() functions but they should be fairly self explanatory. The strides function is a very nice feature since if a user calls the template as such <type, 1,3,5> giving you a 1x3x5 Matrix; these are stored in the _strides member vector. This way you always have the indexes needed for the size of each dimension.

Now if you want your matrix on the heap; instead of trying to do a double pointer or [][] and putting each element on the heap, with this class you have two options.

You can either put the instantiated object on the heap directly or you can have this class hold heap objects as well.

std::shared_ptr<Matrix<int,2,2>> ptrMat2x2; // A heap pointer of the matrix
Matrix<shared_ptr<int>,3,3> mat3x3ptrs; // A matrix of heap objects.

The code might seem a bit strange at first glance but this shows that both cases can be done:

#include <iostream>
#include "Matrix.h"

int main() {
    // A Matrix<> on the heap via shared_ptr`
    std::shared_ptr<Matrix<int, 2, 2>> ptrMat2x2 =
      std::make_shared<Matrix<int, 2, 2>>( Matrix<int,2,2>( 1, 2, 3, 4 ) );

    // accessing the elements from the shared pointer and printing
    for( int i = 0; i < 4; i++ ) 
        std::cout << (*ptrMat2x2.get())(i) << ' ';
    std::cout << '\n';

    // creating some basic shared_ptrs 
    auto a = std::make_shared<int>( 1 );
    auto b = std::make_shared<int>( 2 );
    auto c = std::make_shared<int>( 3 );
    auto d = std::make_shared<int>( 4 );
    // Matrix that holds shared_ptrs
    Matrix<std::shared_ptr<int>, 2, 2> mat2x2ptrs( a, b, c, d );

    // print the elements from the matrix (don't forget to dereference).
    for( int i = 0; i < 4; i++ )
        std::cout << *mat2x2ptrs[i].get() << ' ';
    std::cout << '\n';

    std::cout << "\nPress any key and enter to quit.\n";
    std::cin.get();
    return 0;
}
Francis Cugler
  • 7,788
  • 2
  • 28
  • 59
  • Other than not being necessary in C++, there really is nothing wrong with the OP's code in the deletion function. It is advantageous in that there is no need to know the number of rows. All that matters is that the allocations are done in the manner the OP is doing the allocations. One allocation for the row pointers, and another for the entire data. If the number of columns is uniform, then reducing the number of allocations / deallocations to 2 calls, instead of `number_of_rows + 1` calls is also an advantage. The other advantage is that the data is contiguous, not spread out. – PaulMcKenzie Apr 04 '18 at 02:12
  • @PaulMcKenzie yeah that does make sense; but I was having trouble wrapping my mind around how to call `[] delete` for both the rows and cols from just the fact that `float** M` was being passed without knowing the matrix's dimensions. Initially it appeared to me to be an x/y problem. – Francis Cugler Apr 04 '18 at 02:27
  • @PaulMcKenzie so if you do have `[i][j]` on the heap. Then you don't have to go through each `[j]` to delete each array on the heap before deleting the outer array of pointers? Another words: calling `delete [] M[0]` will do this for your? – Francis Cugler Apr 04 '18 at 02:30
  • If you have the entire data on the heap in one allocation, then yes, there is no need to loop. See [this answer](https://stackoverflow.com/questions/21943621/how-to-create-a-contiguous-2d-array-in-c/21944048#21944048) for a better explanation. – PaulMcKenzie Apr 04 '18 at 02:34
  • @PaulMcKenzie Okay that does clear things up a bit about how the answer showed removing the pool then the pointers. I think this is starting to come back to me now from after I learned C & pointers and first began learning about C++ around 2001 - 2003...; it's just that I'm more prone to using either `vector`, `vector`, etc. – Francis Cugler Apr 04 '18 at 03:01
  • Thank you so much for your incredibly detailed answer. I'll certainly take a look at your class as it might be helpful in the future. As for the code being the same, I copy-pasted the identical thing when the second one should have just deleted M. It makes sense to have to delete both, since two new[] statements were used. But why does it work without deleting M[0]? I say "work" without actually knowing if it does, since all i get is no errors, but I'm not sure how to tell if there's a leak in the first place. – qp212223 Apr 04 '18 at 05:41
  • @RichardGroenewald Here's the thing with `new` & `delete` & `new []` & `delete []`. For each `new` or `new[]` there has to be a matching `delete` or `delete[]`. Even if you just simply call `delete []` on the outer array of pointers, yes it will compile but it can lead to undefined behavior. What happens here is that you are freeing the memory of the external array deleting all the pointers that point to pointers. However, you're failing to delete all the inner pointers that are pointing to something. This leads to what is called dangling pointers, and is a form of a memory leak. – Francis Cugler Apr 04 '18 at 07:52
-1

You have allocated two separate arrays, you need to delete two separate arrays, in reverse order.

bipll
  • 11,747
  • 1
  • 18
  • 32