Templated Matrix class

Question

I created a Matrix class. I tested it and it seems to work.

1. Can someone tell me if this class is good?
2. Can I improve it?
3. Can I use more move semantics (where)? What do I have to modify?
4. Are there some logic/programming errors?

#ifndef MATRIX_H
#define MATRIX_H

#include <iostream>
#include <initializer_list>
#include <stdexcept>
#include <utility>
#include <type_traits>

template <typename T>
class Matrix
{
    static_assert(std::is_arithmetic<T>::value,"");

public:
    Matrix(size_t, size_t);
    Matrix(size_t, size_t, const T&);
    Matrix(const Matrix<T>&);
    virtual ~Matrix();

    void set(const T&);
    size_t get_row() const;
    size_t get_col() const;

    void print(std::ostream&) const;

    Matrix<T>& operator=(const Matrix<T>&);
    T& operator()(size_t, size_t);
    T operator()(size_t, size_t) const;
    bool operator==(const Matrix<T>&) const;
    bool operator!=(const Matrix<T>&) const;
    Matrix<T>& operator+=(const Matrix<T>&);
    Matrix<T>& operator-=(const Matrix<T>&);
    Matrix<T> operator+(const Matrix<T>&) const;
    Matrix<T> operator-(const Matrix<T>&) const;
    Matrix<T>& operator*=(const T& v);
    Matrix<T>& operator*=(const Matrix<T>&);
    Matrix<T> operator*(const Matrix<T>&) const;

private:
    size_t row;
    size_t col;
    size_t dim;
    T* data;

    void copy(const Matrix<T>&);
};

template <typename T>
inline Matrix<T>::Matrix(size_t r, size_t c)
: row(r), col(c), dim(r*c), data(new T[r*c])
{}

template <typename T>
inline Matrix<T>::Matrix(size_t r, size_t c, const T& v)
: row(r), col(c), dim(r*c), data(new T[r*c])
{
    set(v);
}

template <typename T>
inline Matrix<T>::Matrix(const Matrix<T>& m)
{
    copy(m);
}

template <typename T>
inline Matrix<T>::~Matrix()
{
    delete [] data;
}

template <typename T>
inline void Matrix<T>::set(const T& v)
{
    for(size_t i(0); i < dim; i++)
    {
        data[i] = v;
    }
}

template <typename T>
inline size_t Matrix<T>::get_row() const
{
    return row;
}

template <typename T>
inline size_t Matrix<T>::get_col() const
{
    return col;
}

template <typename T>
inline void Matrix<T>::print(std::ostream& out) const
{
    for(size_t i(0); i < row; i++)
    {
        for(size_t j(0); j < col; j++)
        {
            out << data[col*i + j] << ' ';
        }

        out << std::endl;
    }
}

template <typename T>
inline Matrix<T>& Matrix<T>::operator=(const Matrix<T>& m)
{
    if(this != &m)
    {
        delete [] data;
        copy(m);
    }

    return *this;
}

template <typename T>
inline T& Matrix<T>::operator()(size_t i, size_t j)
{
    if( (i == 0) or (j == 0) or (i > row) or (j > col))
    {
        throw std::out_of_range("");
    }

    return data[col*(i-1) + (j-1)];
}

template <typename T>
inline T Matrix<T>::operator()(size_t i, size_t j) const
{
    if( (i == 0) or (j == 0) or (i > row) or (j > col))
    {
        throw std::out_of_range("");
    }

    return data[col*(i-1) + (j-1)];
}

template <typename T>
inline bool Matrix<T>::operator==(const Matrix<T>& m) const
{
    if( (row != m.row) or (col != m.col) )
    {
        return false;
    }

    for(size_t i(0); i < row; i++)
    {
        if(data[i] != m.data[i])
        {
            return false;
        }
    }

    return true;
}

template <typename T>
inline bool Matrix<T>::operator!=(const Matrix<T>& m) const
{
    return !((*this) == m);
}

template <typename T>
inline Matrix<T>& Matrix<T>::operator+=(const Matrix<T>& m)
{
    if( (row != m.row) or (col != m.col) )
    {
        throw std::range_error("");
    }

    for(size_t i(0); i < dim; i++)
    {
        data[i] += m.data[i];
    }

    return *this;
}

template <typename T>
inline Matrix<T>& Matrix<T>::operator-=(const Matrix<T>& m)
{
    if( (row != m.row) or (col != m.col) )
    {
        throw std::range_error("");
    }

    for(size_t i(0); i < dim; i++)
    {
        data[i] -= m.data[i];
    }

    return *this;
}

template <typename T>
inline Matrix<T> Matrix<T>::operator+(const Matrix<T>& m) const
{
    if( (row != m.row) or (col != m.col) )
    {
        throw std::range_error("");
    }

    Matrix<T> tmp(*this);

    for(size_t i(0); i < dim; i++)
    {
        tmp.data[i] += m.data[i];
    }

    return std::move(tmp);
}

template <typename T>
inline Matrix<T> Matrix<T>::operator-(const Matrix<T>& m) const
{
    if( (row != m.row) or (col != m.col) )
    {
        throw std::range_error("");
    }

    Matrix<T> tmp(*this);

    for(size_t i(0); i < dim; i++)
    {
        tmp.data[i] -= m.data[i];
    }

    return std::move(tmp);
}

template <typename T>
inline Matrix<T>& Matrix<T>::operator*=(const T& v)
{
    for(size_t i(0); i < dim; i++)
    {
        data[i] *= v;
    }

    return *this;
}

template <typename T>
inline Matrix<T>& Matrix<T>::operator*=(const Matrix<T>& m)
{
    if( col != m.row )
    {
        throw std::range_error("");
    }

    Matrix<T> tmp(*this);

    col = m.col;
    delete [] data;
    data = new T[row*col];

    for(size_t i(0); i < row; i++)
    {
        for(size_t j(0); j < m.col; j++)
        {
            for(size_t k(0); k < tmp.col; k++ )
            {
                data[col*i + j] += tmp.data[tmp.col*i + k] * m.data[m.col*k + j];
            }
        }
    }

    return *this;
}

template <typename T>
inline Matrix<T> Matrix<T>::operator*(const Matrix<T>& m) const
{
    if( col != m.row )
    {
        throw std::range_error("");
    }

    Matrix<T> tmp(*this);

    return std::move(tmp *= m);
}

template <typename T>
inline void Matrix<T>::copy(const Matrix<T>& m)
{
    row = m.row;
    col = m.col;
    dim = m.dim;

    data = new T[dim];

    for(size_t i(0); i < dim; i++)
    {
        data[i] = m.data[i];
    }
}

template <typename T>
inline Matrix<T> operator*(const T& v, const Matrix<T>& m)
{
    Matrix<T> tmp(m);

    tmp *= v;

    return std::move(tmp);
}

template <typename T>
inline Matrix<T> operator*(const Matrix<T>& m, const T& v)
{
    return std::move(v * m);
}

template <typename T>
inline std::ostream& operator<<(std::ostream& out, const Matrix<T>& m)
{
    m.print(out);

    return out;
}

#endif

Answer 1

I would make several improvements.

I really don't like the fact that the interface has no names on parameters. You can skip all the names you want on the definitions, but when I look at a declaration I want to know what the parameters are.

Still on the interface part, I'd rename get_row and get_col to rows and columns: they don't get you a row or a column, but the total number of rows or columns. set I would probably name fill, but then I'd remember there's a std::fill algorithm already in the <algorithm> header. That would lead me to think about providing an iterator-based interface: it brings the ability to use all the standard algorithms. I'll come back to this point later.

Then, I see no reason to provide a virtual destructor. Why would Matrix be a polymorphic base class? If you use it polymorphically you lose the ability to properly pass it by value, and that makes the overloaded operators add confusion and opportunities for slicing.

Unless you are using the Visual Studio compiler (which has this known issue), there is no need to move local variables in return statements like return std::move(tmp);, or even to move temporaries like return v * m;. The compiler does that automatically. Making the move explicit causes confusion, at least to those that are already acquainted with the C++11 features.

The biggest change I'd make would be to follow what I call the rule of zero. I'd use an existing solution for handling memory ownership. In this case, a std::vector member. That means I don't need to write a copy constructor, nor a copy assignment operator, nor a destructor: I get all those for free and a move constructor and a move assignment operator. Less code and more features.

And now that I have a std::vector member, I can use it to easily provide an iterator based interface: just return iterators from the vector, and everything works.

Answer 2

One general comment is that, instead of catching dimensional mis-matches in assignments and operations between matrices at runtime, you can add two more template parameters for the number of rows and columns respectively. Then you can catch these mis-matches at compile time. I have done this in two matrix implementations in the past and it works quite well.

template <typename T, unsigned int ROWS, unsigned int COLS>
class Matrix { .... };

Of course, this means that different size matrices are different types.