patterncppModerate
Custom matrix class
Viewed 0 times
custommatrixclass
Problem
It took me while to finish, but here is my custom matrix class. I assume that the row/column iterators are the most critical part of this class but anyway I would very much appreciate your ideas of this project.
```
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include "Vector.h"
#ifndef MATRIX_H
#define MATRIX_H
template
class Matrix{
private:
// row and/or colsize must be greater then 0;
static_assert(rowsize > 0, "Number of Rows must be greater then 0.");
static_assert(colsize > 0, "Number of Columns must be greater then 0.");
// Data is stored in a MVector, a modified std::vector
MVector matrix;
// size of row dimension of the matrix
std::size_t row_dim;
// size of row dimension of the matrix
std::size_t column_dim;
public:
// *
// Constructors
// *
// default constructor
// @ param "rowsize" - row dimension of matrix
// @ param "colsize" - column dimension of matrix
Matrix() :matrix(rowsize * colsize),
row_dim(rowsize),
column_dim(colsize) {}
// initializer list constructor
// allows to create a matrix by an initialiser list
// example: Matrix a_matrix = {1,2,3,4};
// @param "il" - initializer list
Matrix(std::initializer_list il) :matrix(il),
row_dim(rowsize),
column_dim(colsize){}
// constructor by MVector
// if the size of the vector is larger then the size of the matrix,
// the matrix will construct with the first rowsize*colsize elements of the vector
// if the size of the vector is smaller then the size of the matrix,
// the matrix will not construct
explicit Matrix(const MVector& s): matrix(s),
row_dim(rowsiz
```
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include "Vector.h"
#ifndef MATRIX_H
#define MATRIX_H
template
class Matrix{
private:
// row and/or colsize must be greater then 0;
static_assert(rowsize > 0, "Number of Rows must be greater then 0.");
static_assert(colsize > 0, "Number of Columns must be greater then 0.");
// Data is stored in a MVector, a modified std::vector
MVector matrix;
// size of row dimension of the matrix
std::size_t row_dim;
// size of row dimension of the matrix
std::size_t column_dim;
public:
// *
// Constructors
// *
// default constructor
// @ param "rowsize" - row dimension of matrix
// @ param "colsize" - column dimension of matrix
Matrix() :matrix(rowsize * colsize),
row_dim(rowsize),
column_dim(colsize) {}
// initializer list constructor
// allows to create a matrix by an initialiser list
// example: Matrix a_matrix = {1,2,3,4};
// @param "il" - initializer list
Matrix(std::initializer_list il) :matrix(il),
row_dim(rowsize),
column_dim(colsize){}
// constructor by MVector
// if the size of the vector is larger then the size of the matrix,
// the matrix will construct with the first rowsize*colsize elements of the vector
// if the size of the vector is smaller then the size of the matrix,
// the matrix will not construct
explicit Matrix(const MVector& s): matrix(s),
row_dim(rowsiz
Solution
The first rule of matrix classes in C++ is that you don't write them by yourself; there are already dozens around, often highly optimized with [smart] expression templates and other cryptic stuff: Boost.uBLAS, Eigen, Blaze, Gmm++, Armadillo, Blitz++, etc...
That said, I would lie to you if I told you that I never tried to reimplement my own matrix class. I think that I tried to create one at leat two or three times :)
Static versus dynamic storage
First of all, what I find odd is that your
Useless variables
The variable
By the way, I would expect functions named
Hide the implementation
Your class currently shows too many implementation details in its inteface. For example:
The fact that your class uses an
That way, if you change
Standard library style
As I just said, writing containers that look like those used in the standard library generally makes them compatible with more existing code. Therefore, your functions should be named as in the standard library: instead of
Also, you miss a
This should be a matrix by default, not a vector
Currently, many of the main operations clearly consider the class as a vector and not as a matrix. Those main operations are
Being able to treat the matrix as a vector is often useful, but it should be explicitly required by the user, not the default option. If you want an example of implementation, the last matrix class I wrote provided the methods
That said, I would lie to you if I told you that I never tried to reimplement my own matrix class. I think that I tried to create one at leat two or three times :)
Static versus dynamic storage
First of all, what I find odd is that your
Matrix class, whose size is known at compile time, is built on the top of a MVector that is itself built on the top of an std::vector. That means that the memory is allocated of the heap and that your class will probably be much slower than intended. Since the size is known at compile time, you could have buit your class on the top of an std::array instead, which has way less overhead and should allocate its memory on the stack.Useless variables
The variable
row_dim and column_dim are useless. They are known at compile time and the information is already in Matrix's template parameters rowsize and colsize (it would be good to capitalize template parameters by the way). You could get rid of these variables and reimplement the functions using them this way:// returns the number of rows of the matrix
constexpr std::size_t rows() const{
return rowsize;
}
// returns the number of colums of the matrix
constexpr std::size_t cols() const{
return colsize;
}By the way, I would expect functions named
cols and rows to return columns and rows, not their size; you way want to change their names. You could also implement size so that it is also available at compile time:constexpr std::size_t size() const{
return cols() * rows();
}Hide the implementation
Your class currently shows too many implementation details in its inteface. For example:
typename std::vector::iterator Begin(){
return matrix.Begin();
}The fact that your class uses an
std::vector is an implementation detail. You should have written type aliases in Matrix and in MVector to provide a better abstraction:// In Matrix
using iterator = typename MVector::iterator;
using const_iterator = typename MVector::const_iterator;That way, if you change
std::vector to something else in MVector, you won't have to update Matrix accordingly. It would also be a good idea to add other common subtypes such as value_type, pointer, reference... as in the standard library containers. A lot of template code out there relies on such subtypes.Standard library style
As I just said, writing containers that look like those used in the standard library generally makes them compatible with more existing code. Therefore, your functions should be named as in the standard library: instead of
Begin and End, uncapitalized begin and end would have been better alternatives (that's actually why I try to write my code with the same case as the one used in the standard library). The full-lowercase member begin and end will work with both C++11's std::begin and std::end and with the range-based for loop.Also, you miss a
const overload for Begin and End.This should be a matrix by default, not a vector
Currently, many of the main operations clearly consider the class as a vector and not as a matrix. Those main operations are
(c)begin, (c)end, operator[] and somehow the constructors. For a "matrix", I would expect operator[] to return a row or a column (it depends whether the matrix is row-major or column-major, documenting that would help) and (c)begin/(c)end to help iterating over rows or columns.Being able to treat the matrix as a vector is often useful, but it should be explicitly required by the user, not the default option. If you want an example of implementation, the last matrix class I wrote provided the methods
fbegin and fend that returned flat_iterators. I also wrote a container view, flat, so that I could write this:matrix mat = { /* ... */ };
for (int& val: flat(mat))
{
val *= 5;
}Code Snippets
// returns the number of rows of the matrix
constexpr std::size_t rows() const{
return rowsize;
}
// returns the number of colums of the matrix
constexpr std::size_t cols() const{
return colsize;
}constexpr std::size_t size() const{
return cols() * rows();
}typename std::vector<T>::iterator Begin(){
return matrix.Begin();
}// In Matrix
using iterator = typename MVector<T>::iterator;
using const_iterator = typename MVector<T>::const_iterator;matrix<int, 5, 4> mat = { /* ... */ };
for (int& val: flat(mat))
{
val *= 5;
}Context
StackExchange Code Review Q#68486, answer score: 11
Revisions (0)
No revisions yet.