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