Basic Templated Matrix Library

I am a beginner C++ programmer and I have been working on a library to use for working with matrices. The library main features should be

• Working with matrices (addition, subtraction, arbitrary operations)

• Lazy Evaluation and Reference Counting

• Modifiable underlying container

Of course, all of this trying to maintain an acceptable performance.

Any feedback is appreciated, however I am in particular interested in knowing about efficiency issues, violated design principles and any red flags I may have missed.

The structure of the code is the following:

• Matrix.hpp and MatrixFactory.hpp are responsible for creating and

operating on a Matrix where T is the type of the
elements in the matrix, and container the underlying container

• MatrixLazyEvaluator.hpp is responsible for almost all of the lazy

evaluation

• RCObject.hpp, RCObject.cpp, RCPtr.hpp are responsible for

Reference Counting, and they are shamelessly stolen from 'More
Effective C++'

• MatrixStorage.hpp, DynamicMatrixStorage.hpp,

StaticMatrixStorage.hpp are the underlying containers that we use
for Matrix. Any user wishing to provide their own implementation will
need to implement MatrixStorage.hpp

• Array_Wrapper.hpp is simply a wrapper for T[] (std::array was

not suitable because of the compile time size)

• main.cpp contains some tests to show intended behaviour

GitHub Repo (MML)

Matrix.hpp

```
#pragma once

#include
#include
#include

#include "MatrixStorage.hpp"
#include "RCObject.hpp"
#include "RCPtr.hpp"
#include "MatrixLazyEvaluator.hpp"

namespace MML
{

template >
class Matrix
{
public:
explicit Matrix(MatrixStorage* storage) noexcept :
_matrix_value{ new MatrixValue(storage) }, _lazy(this) {}

class ValueProxy
{
public:
ValueProxy(Matrix& matrix, const size_t& row, const size_t& col);

ValueProxy& operator=(const ValueProxy& rhs); // WRITE
ValuePro

Here are my thoughts:

-
Even though #pragma once is support by every major compiler (VC, gcc, clang and icc), it is a language extension. You should try to write the most standard compliant code possible, so #ifndef guards would be better.

-
Your code doesn't compile for me with gcc. That's because (1) you are using std::size_t without including a header that defines it (like `) and (2) std::size_t is in std, not in the global namespace.

-
Fixing the previous errors, your code still doesn't compile, and that's because you cannot use parameters of a function for default arguments of the same function:

void foo(int a, int = a); // illegal

To fix this, define a second function:

void foo(int a) { foo(a, a); }
void foo(int a, int) { /* ... */ }

-
That's probably VS's fault, but gcc emits this warning:

warning: base class 'class RCObject' should be explicitly initialized in the copy constructor [-Wextra]
     MatrixValue(const MatrixValue& rhs) :

Always initialize the base class in the derived class's constructor.

-
Always make your code compile with no warnings on high (but not aggressive) levels of warnings. You have a lot of parameters that you don't use, i.e. argc, argv and rhs. Consider omitting their names or in the case of main, just remove them.

-
You are not using the copy and swap idiom.

-
A destructor who does nothing should be defaulted:

virtual RCObject() = default;

-
Use auto to save a lot of characters!

template 
typename MatrixStorage::iterator MatrixStorage::end()
{
    using std::end;
    return end(_arr);
}

becomes

template 
auto MatrixStorage::end()
{
    using std::end;
    return end(_arr);
}

way more readable!

-
Returning mutable iterators from a const container is not good and can lead to undefined behavior. The begin and end functions shouldn't be marked const, and the cbegin and cend should be marked const. Not some other permutations of constness.

-
I don't get the point of Array_Pointer. Isn't a std::vector enough? It is an array allocated on the heap, just like you want.

-
In some classes, you are not implementing the Rule of Five.

-
Again, why did you make your own shared pointer? Why not use std::shared_ptr? It works flawlessly with only 1 modification to your code

-
RCPtr is useless (you're not using it anywhere), you can remove it safely.

-
Declare your classes as final if they shouldn't be derived from.

-
Don't use new/delete please. Instead, use std::unique_ptr and co.

-
I think it would be better if you hid the storage inside of Matrix, and implemented everything using the constructors, without any factory. To specify the storage type, use the Policy Pattern, the same that you used for the container.

I don't like factories, for the only reason that in my opinion they are completely unnecessary. You can just use the constructor of the class the factory creates and that's it. But in your case, that's not possible, because the user has to pass a storage object, fully initialized. But why does the user have to do this? This is supposed to be a library detail, which only Matrix should have access to (and should own), not the user. This leads to all kinds of problems.

But there still has to be a way to specify the storage type, right? Yes, this is where the policy pattern kicks in. Actually, you have already done it with the container parameter: The policy pattern just states that you can modify a class as you please (something like that). By passing template parameters, you can choose which behavior you want the class to have for different policies. Which container? How are the object allocated? How are they stored? Those things can be customized using policies:

template
class Matrix {};

That way, the user can choose which storage they want, without actually needing to initialize it, that's Matrix`'s job.

StackExchange Code Review Q#152241, answer score: 4