4
\$\begingroup\$

This is my 2nd shot at dynamic memory allocation. This project is for practice purposes. So many things were considered whilst writing this minimal project.

  1. I considered using placement new to dynamically allocate memory, this would be optimal for large objects. I finally resolved to restrict user of my class to c++ built-in types
  2. I considered having commutative arithmetic operators. I finally decided that 2 + mat makes no sense
  3. I considered supporting range checking, after much consideration, I decided that users of my class should be more careful :-)
  4. I considered making co_factor, det, transpose, swap, valid_dim member-functions. I finally decided that those functions do not need access to elem_ so I removed them from the interface.

The following are some areas I hope to get reviews on and as such improve upon.

  1. Design
  2. Performance
  3. Ease of use

Note: The code is a little large.

Matrix.h


#ifndef MATRIX_H_
#define MATRIX_H_

#include <algorithm>
#include <cstdlib>
#include <initializer_list>
#include <iostream>

namespace mat
{
    template<typename T>
    void swap(T& a, T& b);

    bool valid_dim(int row, int column);

    template<class T>
    class Matrix
    {
        public:
            explicit Matrix(int row = 1, int column = 1, const T& val = {})
                : row_{row}, column_{column}
                {
                    if(!valid_dim(row_, column_))
                        throw std::invalid_argument("Exception: Invalid row and column in constructor");
                    
                    elem_ = new T[ row_ * column_];

                    for(size_t i = 0; i != size(); ++i )
                        elem_[i] = val;
                }
            
            Matrix(int row, int column, std::initializer_list<T> list)
                : row_{row}, column_{column} 
                {
                    if(!valid_dim(row_, column_))
                        throw std::invalid_argument("Exception: Invalid row and column in constructor");

                    if(list.size() != size())
                        throw std::runtime_error("Exception: Intializer list argument does not match Matrix size in constructor");

                    elem_ = new T[ row_ * column_];

                    int i = 0;
                    for(const auto& item : list)
                    {
                        elem_[i] = item;
                        ++i;
                    }
                }
            
            Matrix(const Matrix& M)
            :  row_{M.row_}, column_{M.column_}, elem_{new T[ row_ * column_]} 
            {
               std::copy(M.elem_, M.elem_+size(), elem_);
            }

            Matrix& operator=(const Matrix& M)
            {
                if(row_ != M.row_ || column_ != M.column_)
                    throw std::runtime_error("Exception: Unequal size in Matrix=");
                row_ = M.row_;
                column_ = M.column_;

                std::copy(M.elem_, M.elem_ + size(), elem_);
                
                return *this;
            }

            Matrix(Matrix&& M) noexcept
            : row_{0}, column_{0}, elem_{nullptr}
            {
                swap(row_, M.row_);
                swap(column_, M.column_);
                swap(elem_, M.elem_);
            }

            Matrix& operator=(Matrix&& M) noexcept
            {
                swap(row_, M.row_);
                swap(column_, M.column_);
                swap(elem_, M.elem_);

                return *this;
            }

            ~Matrix() { delete [] elem_; }

            T& operator()(const int i, const int j) { return elem_[i * column_ + j]; } // Note: no range checking
            const T& operator()(const int i, const int j) const { return elem_[i * column_ + j]; }

            size_t size() const { return row_ * column_; }
            size_t row() const { return row_; }
            size_t column() const { return column_; }

            Matrix& operator+=(const Matrix& rhs)
            {
                Matrix res = (*this) + rhs;
                *this = res;

                return *this;
            }

            Matrix& operator-=(const Matrix& rhs)
            {
                Matrix res = (*this) - rhs;
                *this = res;

                return *this;
            }

            Matrix& operator*=(const Matrix& rhs)
            {
                Matrix res = (*this) * rhs;
                *this = res;

                return *this;
            }

            Matrix& operator+=(const double rhs)
            {
                Matrix res = (*this) + rhs;
                *this = res;

                return *this;
            }

            Matrix& operator-=(const double rhs)
            {
                Matrix res = (*this) - rhs;
                *this = res;

                return *this;
            }

            Matrix& operator*=(const double rhs)
            {
                Matrix res = (*this) * rhs;
                *this = res;

                return *this;
            }

            Matrix& operator/=(const double rhs)
            {
                Matrix res = (*this) / rhs;
                *this = res;

                return *this;
            }

            Matrix operator+(const Matrix& rhs)
            {
                if(row_ != rhs.row_ || column_ != rhs.column_)
                    throw std::runtime_error("Exception: Unequal size in Matrix+");
                
                Matrix res(row_, column_, 0.0);

                for(size_t i = 0; i != size(); ++i)
                {
                    res.elem_[i] = elem_[i] + rhs.elem_[i];
                }

                return res;
            }
            Matrix operator-(const Matrix& rhs)
            {
                if(row_ != rhs.row_ || column_ != rhs.column_)
                    throw std::runtime_error("Exception: Unequal size in Matrix-");
                
                Matrix res(row_, column_, 0.0);

                for(size_t i = 0; i != size(); ++i)
                {
                    res.elem_[i] = elem_[i] - rhs.elem_[i];
                }

                return res;
            }

            Matrix operator*(const Matrix& rhs)
            {
                if(row_ != rhs.column_ )
                    throw std::runtime_error("Exception: Unequal size in Matrix*");
                Matrix res(rhs.row_, column_, 0.0);
                for(int i = 0; i != rhs.row_; ++i)
                {
                    for(int j = 0; j != column_; ++j)
                    {
                        for(int k = 0; k != row_; ++k)
                        {
                            res(i, j) += rhs(i, k) * this->operator()(k, j);
                        }
                    }                        
                }
                
                return res;
            }

            Matrix operator+(const double rhs)
            {
                Matrix res(row_, column_, 0.0);

                for(size_t i = 0; i != size(); ++i)
                    res.elem_[i] = elem_[i] + rhs;

                return res;
            }

            Matrix operator-(const double rhs)
            {
                Matrix res(row_, column_, 0.0);

                for(size_t i = 0; i != size(); ++i)
                    res.elem_[i] = elem_[i] - rhs;

                return res;
            }

            Matrix operator*(const double rhs)
            {
                Matrix res(row_, column_, 0.0);

                for(size_t i = 0; i != size(); ++i)
                    res.elem_[i] = elem_[i] * rhs;

                return res;
            }

            Matrix operator/(const double rhs)
            {
                Matrix res(row_, column_, 0.0);

                for(size_t i = 0; i != size(); ++i)
                    res.elem_[i] = elem_[i] / rhs;

                return res;
            }

        private:
            int row_;
            int column_; 
            T *elem_;
    };

    template<typename T>
    inline void swap(T& a, T& b)
    {
        const T tmp = std::move(a);
        a = std::move(b);
        b = std::move(tmp);
    }

    inline bool valid_dim(int row, int column) { return (row >= 1 || column >= 1); }

    template<typename T>
    Matrix<T> transpose(const Matrix<T>& A)
    {
        Matrix<T> res(A.column(), A.row(), 0.0);
        for(size_t i = 0; i != res.row(); ++i)
        {
            for(size_t j = 0; j != res.column(); ++j)
            {
                res(i, j) = A(j, i);
            }
        }
        return res;
    }

    template<typename T>
    Matrix<T> co_factor(const Matrix<T>& A, size_t p, size_t q)
    {
        if(p >= A.row() || q >= A.column())
            throw std::invalid_argument("Exception: Invalid argument in cofactor(int, int)");

        if(A.row() != A.column())
            throw std::runtime_error("Exception:Unequal row and column in co_factor(int, int)");
        
        Matrix<T> res(A.row() - 1, A.column() - 1);
        size_t a = 0, b = 0;

        for(size_t i = 0; i != A.row(); ++i)
        {
            for(size_t j = 0; j != A.column(); ++j)
            {
                if(i == p || j == q)
                    continue;

                res(a, b++) = A(i, j);
                if(b == A.column() - 1)
                {
                    b = 0;
                    a++;
                }
            }
        }

        return res;
    }

    template<typename T>
    double det(const Matrix<T> A)
    {
        if(A.row() != A.column())
            throw std::runtime_error("Exception:Unequal row and column in det()");
        
        if(A.row() == 2)
        {
            return ( A(0,0) * A(A.row()-1,A.row()-1) ) - ( A(0,1) * A(A.row()-1,A.row()-2) );
        }

        int sign = 1, determinant = 0;
        for(size_t i = 0; i != A.row(); ++i)
        {
            Matrix<T> co_fact = co_factor(A, 0, i);
            determinant += sign * A(0, i) * det(co_fact);
            sign = -sign;
        }
        
        return determinant;
    }
}

#endif

main.cpp

#include <iostream>
#include "Matrix.h"

using namespace mat;

template<typename T>
void display(const T& A)
{
    for(size_t i = 0; i != A.row(); ++i)
    {
        for(size_t j = 0; j != A.column(); ++j)
        {
            std::cout << A(i, j) << " ";
        }
        std::cout << '\n';
    }
}

int main()
{
    Matrix<double> my_mat1(2,2, {1,2,3,4});
    Matrix<double> my_mat2(2,2, {5,6,7,8});
    std::cout << "\nDisplay matrix: \n";
    display(my_mat1);
    std::cout << '\n';
    display(my_mat2);
    std::cout << "\nAddition: \n";
    display(my_mat1 + my_mat2);
    std::cout << "\nSubtraction: \n";
    display(my_mat2 - my_mat1);
    std::cout << "\nMultiplication: \n";
    display(my_mat1 * my_mat2);

    std::cout << "\nInplace Addition: \n";
    my_mat1 += my_mat2;
    display(my_mat1);
    std::cout << "\nInplace Subtraction: \n";
    my_mat1 -= my_mat2;
    display(my_mat1);
    std::cout << "\nInplace Multiplication: \n";
    my_mat1 *= my_mat2;
    display(my_mat1);

    std::cout << "\nTranspose: \n";
    display(transpose(my_mat2));

    std::cout << "\nAdding 2 to my_mat1: \n";
    my_mat1 += 2;
    display(my_mat1);

    Matrix<int> my_mat3 {4,4,
    {
        1,0,2,-1,
        3,0,0,5,
        2,1,4,-3,
        1,0,5,0
    }};
    
    Matrix<double> co_factor_mat = co_factor(my_mat1, 0, 0);
    std::cout << "\nCofactor: \n";
    display(co_factor_mat);
    std::cout << "Determinant of matrix: " << det(my_mat3) << std::endl;

}

\$\endgroup\$
3
\$\begingroup\$
  • Use an unsigned type for rows and columns (probably std::size_t).
  • Creating a size 1 matrix as the default (empty constructor args) probably isn't useful behavior. We could have an ordinary default constructor creating a zero-sized matrix.
  • valid_dim is wrong (|| should be &&) and unnecessary if we use an unsigned type for rows and columns. (There's nothing wrong with allocating a zero size array in C++, or we could set elem_ to nullptr).
  • We can use std::fill in the value constructor and std::copy in init list constructor.
  • It's strange to prevent assignment from a different sized matrix (and very unexpected for the user for it to throw). We should just resize the matrix if necessary.
  • We can provide an at(i,j) function that does size checking (similar to the standard library containers).
  • We'd normally implement the binary math operators (+, -, etc.) using the math-assignment operators (+=, -=, etc.); the opposite of how they are implemented above. (The assignment versions can modify the values in place).
  • 2.0 * m is as reasonable as m * 2.0 - we should implement that too. Usually we'd implement binary math operators as free functions using +=, -= etc. where possible (and it's only one more line of code where we can't).
  • swap can't std::move out of the const tmp variable - it'll always copy.
  • Note that we don't need to write a custom swap function - std::swap will do exactly the same thing by default.
  • There are quite a lot of other useful functions we could implement (c.f. the standard library containers): empty(), clear(), resize(), data(), iterators (begin(), rbegin(), etc.), operator== and operator!=.
  • I guess this is an exercise in manual memory management, but we really should use std::vector for storage! Implementing everything becomes much easier.
\$\endgroup\$
2
  • \$\begingroup\$ Keeping unsigned for rows and columns might lead to conversion from -1 to 4294967295. This might not be what the user expected. \$\endgroup\$ Nov 24 '20 at 14:01
  • 1
    \$\begingroup\$ Well... that's how unsigned arithmetic works in C++, and we have to assume the user has a basic level of competency in the language. (It's how all the standard library containers do things, so it's at least consistent). \$\endgroup\$
    – user673679
    Nov 24 '20 at 16:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.