Matrix implementation

by

I am trying to implement an optimal and fast running matrix in C++. I need some review of the code and ideas on how to improve the code quality if it shall be.

    class Matrix {

        // Some static assertions and useful types
        static_assert(std::is_arithmetic_v<T>, "Matrix template parameter type must be arithmetic");
        using DataType = std::vector<T>;

        // Default constructors
    public:
        Matrix() : mCols(0), mRows(0), mData(0) { };
        Matrix(const Matrix &other) = default;
        Matrix(Matrix &&other) noexcept = default;
        Matrix &operator=(const Matrix &other) = default;
        Matrix &operator=(Matrix &&other) noexcept = default;

        // Parameterized constructors
    public:
        Matrix(std::size_t cols, std::size_t rows) : mCols(cols), mRows(rows) {
            mData.resize(cols * rows);
        }
        Matrix(std::size_t cols, std::size_t rows, const DataType &data) {
            if (rows * cols != data.size()) {
                throw std::invalid_argument("Invalid matrix mData");
            }
            mCols = cols;
            mRows = rows;
            mData = data;
        }
        Matrix(std::size_t cols, std::size_t rows, DataType &&data) {
            if (rows * cols != data.size()) {
                throw std::invalid_argument("Invalid matrix mData");
            }
            mCols = cols;
            mRows = rows;
            mData = std::move(data);
        }

        // Setters
    public:
        void set(std::size_t rows, std::size_t cols, const DataType &data) {
            if (rows * cols != data.size()) {
                throw std::invalid_argument("Invalid vector data");
            }
            mRows = rows;
            mCols = cols;
            mData.resize(rows * cols);
            std::copy(data.begin(), data.end(), mData.begin());
        }
        void set(std::size_t rows, std::size_t cols, DataType &&data) {
            if (rows * cols != data.size()) {
                throw std::invalid_argument("Invalid vector data");
            }
            mRows = rows;
            mCols = cols;
            mData.resize(rows * cols);
            std::move(data.begin(), data.end(), mData.begin());
        }
        void set(std::size_t rows, std::size_t cols) {
            mData.resize(rows * cols);
        }
        void set(const DataType &data) {
            if (mRows * mCols != data.size()) {
                throw std::invalid_argument("Invalid vector data");
            }
            std::copy(data.begin(), data.end(), mData.begin());
        }
        void set(DataType &&data) {
            if (mRows * mCols != data.size()) {
                throw std::invalid_argument("Invalid vector data");
            }
            std::move(data.begin(), data.end(), mData.begin());
        }

        // Getters
    public:
        auto cols() const {
            return mCols;
        }

        auto rows() const {
            return mRows;
        }

        const DataType &data() const {
            return mData;
        }

        // Operator overloads
    public:
        Matrix operator+(const Matrix &rhs) const {
            if (mCols != rhs.mCols || mRows != rhs.mRows) {
                throw std::invalid_argument("Invalid vector mData, you may add matrices with same dimensions only");
            }
            Matrix result(mCols, mRows);
            std::transform(mData.begin(), mData.end(), rhs.mData.begin(), result.mData.begin(), std::plus<>());
            return result;
        }
        Matrix operator+(Matrix &&rhs) const {
            if (mCols != rhs.mCols || mRows != rhs.mRows) {
                throw std::invalid_argument("Invalid vector mData, you may add matrices with same dimensions only");
            }
            Matrix result(mCols, mRows);
            std::transform(mData.begin(), mData.end(), std::make_move_iterator(rhs.mData.begin()), result.mData.begin(), std::plus<>());
            return result;
        }

        Matrix operator-(const Matrix &rhs) const {
            if (mCols != rhs.mCols || mRows != rhs.mRows) {
                throw std::invalid_argument("Invalid vector mData, you may sub matrices with same dimensions only");
            }
            Matrix result(mCols, mRows);
            std::transform(mData.begin(), mData.end(), rhs.mData.begin(), result.mData.begin(), std::minus<>());
            return result;
        }
        Matrix operator-(Matrix &&rhs) const {
            if (mCols != rhs.mCols || mRows != rhs.mRows) {
                throw std::invalid_argument("Invalid vector mData, you may sub matrices with same dimensions only");
            }
            Matrix result(mCols, mRows);
            std::transform(mData.begin(), mData.end(), std::make_move_iterator(rhs.mData.begin()), result.begin(), std::minus<>());
            return result;
        }

        Matrix &operator+=(const Matrix &rhs)  {
            if (mCols != rhs.mCols || mRows != rhs.mRows) {
                throw std::invalid_argument("Invalid vector mData, you may add matrices with same dimensions only");
            }
            std::transform(mData.begin(), mData.end(), rhs.mData.begin(), mData.begin(), std::plus<>());
            return *this;
        }
        Matrix &operator+=(Matrix &&rhs)  {
            if (mCols != rhs.mCols || mRows != rhs.mRows) {
                throw std::invalid_argument("Invalid vector mData, you may add matrices with same dimensions only");
            }
            std::transform(mData.begin(), mData.end(), std::make_move_iterator(rhs.mData.begin()), mData.begin(), std::plus<>());
            return *this;
        }

        Matrix &operator-=(const Matrix &rhs)  {
            if (mCols != rhs.mCols || mRows != rhs.mRows) {
                throw std::invalid_argument("Invalid vector mData, you may add matrices with same dimensions only");
            }
            std::transform(mData.begin(), mData.end(), rhs.mData.begin(), mData.begin(), std::minus<>());
            return *this;
        }
        Matrix &operator-=(Matrix &&rhs)  {
            if (mCols != rhs.mCols || mRows != rhs.mRows) {
                throw std::invalid_argument("Invalid vector mData, you may add matrices with same dimensions only");
            }
            std::transform(mData.begin(), mData.end(), std::make_move_iterator(rhs.mData.begin()), mData.begin(), std::minus<>());
            return *this;
        }

        bool operator==(const Matrix &rhs) const {
            return mCols == rhs.mCols && mRows == rhs.mRows && std::equal(mData.begin(), mData.end(), rhs.mData.begin(), rhs.mData.end());
        }
        bool operator!=(const Matrix &rhs) const {
            return !(*this == rhs);
        }


        friend std::ostream &operator<<(std::ostream &os, const Matrix &matrix) {
            for (int i = 0; i < matrix.mCols; ++i) {
                for (int j = 0; j < matrix.mRows; ++j) {
                    os << matrix.mData[i * matrix.mCols + j] << " ";
                }
                os << "\n";
            }
            return os;
        }


    protected:
        std::size_t mCols;
        std::size_t mRows;
        std::vector<T> mData;
    };

I’ll add determinant and matrix multiplication operators too, but that will come after reviewing this part.
Thanks in advance.

Answer

  • for

    Matrix() : mCols(0), mRows(0), mData(0) { };

    semicolon should be removed as useless.

  • for

    Matrix(std::size_t cols, std::size_t rows) : mCols(cols), mRows(rows) {
          mData.resize(cols * rows);
      }

    You can use member list initialization too for vector:

    Matrix(std::size_t cols, std::size_t rows) : mCols(cols), mRows(rows), mData(cols * rows) {}

  • for

    void set(std::size_t rows, std::size_t cols) { mData.resize(rows * cols); }

    it is wrong, as changing row/columns change internal layout, so for example element of 2nd row, 2nd column won’t be there anymore.

    resize seems to be a better name if you keep that function.

  • for arithmetic operators Matrix operator+(const Matrix &rhs) const, you forget when lhs is a rvalue, so either add overload with this qualifier

    Matrix operator+(const Matrix &rhs) &&

    or make all overload as (friend) free functions.

    You might implement operator + with operator +=.

  • std::vector already has an operator == so

    bool operator==(const Matrix &rhs) const {
    return mCols == rhs.mCols && mRows == rhs.mRows && std::equal(mData.begin(), mData.end(), rhs.mData.begin(), rhs.mData.end());
}

    can be simplified to

    bool operator==(const Matrix &rhs) const {
    return mCols == rhs.mCols && mRows == rhs.mRows && mData == rhs.mData;
}

Attribution
Source : Link , Question Author : Hrant Nurijanyan , Answer Author : Jarod42

Leave a Comment