3
\$\begingroup\$

I wanted to make a matrix template class to see if I can learn how to use templates, work on figuring out the indexing in loops, and making an interface so the user can know if an operation will work. Eventually I would like to add more cache optimization, calculating the inverse, and other ways to get data into the matrix such as raw pointers.

For now everything is one header and I will separate out the implementation

#pragma once
#include <cstdint>
#include <vector>
#include <iostream>
#include <iomanip>
#include <algorithm>
#include <functional>
#include <type_traits>

template<typename T>
class LiteMatrix
{
public:
    LiteMatrix(const size_t &rows, const size_t &cols);
    LiteMatrix(const size_t &rows, const size_t &cols, const std::vector<T>&data);
    ~LiteMatrix() = default;

    LiteMatrix(const LiteMatrix &rhs) = default; // copy constructor
    LiteMatrix(LiteMatrix && rhs) = default; // move constructor

    LiteMatrix& operator=(const LiteMatrix& rhs) = default; // copy assignment
    LiteMatrix& operator=(LiteMatrix&& rhs) = default; // move assignment

    LiteMatrix& zeroes();
    LiteMatrix& ones();

    LiteMatrix operator+(const LiteMatrix<T> &rhs);
    LiteMatrix& operator+=(const LiteMatrix<T>& rhs);
    LiteMatrix operator-(const LiteMatrix<T> &rhs);
    LiteMatrix& operator-=(const LiteMatrix<T>& rhs);
    LiteMatrix operator*(const LiteMatrix<T> &rhs);
    LiteMatrix& operator*=(const LiteMatrix<T>& rhs);
    T& operator()(const size_t &rIndex, const size_t &cIndex);

    LiteMatrix operator*(const T &rhs);
    LiteMatrix& operator*=(const T &rhs);

    bool operator!=(const LiteMatrix<T>& rhs) const;
    bool operator==(const LiteMatrix<T>& rhs) const;

    template<typename T>
    friend std::ostream& operator<<(std::ostream& os, const LiteMatrix<T>& rhs);

    size_t rowCount() const;
    size_t colCount() const;

    bool isAddSubLegal(const LiteMatrix& rhs) const;
    bool isMultLegal(const LiteMatrix& rhs) const;

    void setMatrix(const std::vector<T>& val);
    void setElement(const size_t row, const size_t col, const T& val);
    void setRow(const size_t row, const std::vector<T> val);
    void setCol(const size_t col, const std::vector<T> val);
    T getElement(const size_t row, const size_t col) const;

    LiteMatrix& transpose();

private:
    size_t m_rows;
    size_t m_cols;

    std::vector<T> m_mat;
};

//#include "LiteMatrix.tcc"

template<typename T>
LiteMatrix<T>::LiteMatrix(const size_t & rows, const size_t & cols)
    : m_rows(rows), m_cols(cols)
{
    m_mat = std::vector<T>(rows * cols);
}

template<typename T>
LiteMatrix<T>::LiteMatrix(const size_t & rows, const size_t & cols, const std::vector<T>& data)
    : m_rows(rows), m_cols(cols), m_mat(data)
{
}

template<typename T>
LiteMatrix<T>& LiteMatrix<T>::zeroes()
{
    std::fill(m_mat.begin(), m_mat.end(), 0);

    return *this;
}

template<typename T>
LiteMatrix<T>& LiteMatrix<T>::ones()
{
    std::fill(m_mat.begin(), m_mat.end(), 1);

    return *this;
}

template<typename T>
LiteMatrix<T> LiteMatrix<T>::operator+(const LiteMatrix& rhs)
{
    LiteMatrix ret(*this);
    ret += rhs;
    return ret;
}

template<typename T>
LiteMatrix<T>& LiteMatrix<T>::operator+=(const LiteMatrix& rhs)
{
    if (!isAddSubLegal(rhs))
        throw std::range_error("Matrix sizes are not compatible\n");

    std::transform(m_mat.begin(), m_mat.end(), rhs.m_mat.begin(),
        m_mat.begin(), std::plus<>());

    return *this;
}

template<typename T>
LiteMatrix<T> LiteMatrix<T>::operator-(const LiteMatrix& rhs)
{
    LiteMatrix ret(*this);
    ret -= rhs;
    return ret;
}

template<typename T>
LiteMatrix<T>& LiteMatrix<T>::operator-=(const LiteMatrix& rhs)
{
    if (!isAddSubLegal(rhs))
        throw std::range_error("Matrix sizes are not compatible\n");

    std::transform(m_mat.begin(), m_mat.end(), rhs.m_mat.begin(),
        m_mat.begin(), std::minus<>());

    return *this;
}

template<typename T>
LiteMatrix<T> LiteMatrix<T>::operator*(const LiteMatrix& rhs)
{
    LiteMatrix ret(*this);
    ret *= rhs;
    return ret;
}

template<typename T>
LiteMatrix<T>& LiteMatrix<T>::operator*=(const LiteMatrix& rhs)
{
    if (!isMultLegal(rhs))
        throw std::range_error("Matrix index are not compatible\n");

    LiteMatrix<T> temp(m_rows, rhs.m_cols);

    for (size_t i = 0; i < m_rows; i++)
    {
        for (size_t j = 0; j < m_cols; ++j)
        {
            for (size_t k = 0; k < m_cols; ++k)
            {
                temp.m_mat[i * rhs.m_cols + j] += m_mat[i * m_cols + k] * rhs.m_mat[j + k * m_cols];
            }               
        }
    }

    *this = std::move(temp);

    return *this;
}

template<typename T>
LiteMatrix<T> LiteMatrix<T>::operator*(const T& rhs)
{
    LiteMatrix ret(*this);
    ret *= rhs;
    return ret;
}

template<typename T>
LiteMatrix<T>& LiteMatrix<T>::operator*=(const T& rhs)
{
    std::transform(m_mat.begin(), m_mat.end(), m_mat.begin(),
        std::bind(std::multiplies<T>(), std::placeholders::_1, rhs));

    return *this;
}

template<typename T>
T& LiteMatrix<T>::operator()(const size_t& rIndex, const size_t& cIndex)
{
    return m_mat[rIndex * m_cols + cIndex];
}

template<typename T>
bool LiteMatrix<T>::operator!=(const LiteMatrix& rhs) const
{
    bool isNotEqual = false;

    for (size_t i = 0; i < m_rows; i++)
    {
        for (size_t j = 0; j < m_cols; ++j)
        {
            isNotEqual = std::numeric_limits<T>::epsilon() <= 
                std::abs(m_mat[i * m_cols + j] - rhs.m_mat[i * m_cols + j]);
            if (isNotEqual)
                break;
        }
    }
    return isNotEqual;
}

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

template<typename T>
std::ostream& operator<<(std::ostream& os, const LiteMatrix<T>& rhs)
{
    for (size_t i = 0; i < rhs.m_rows; ++i)
    {
        for (size_t j = 0; j < rhs.m_cols; ++j)
            os << std::setw(5) << std::setprecision(2) << rhs.m_mat[i * rhs.m_cols + j] << ' ';

        os << '\n';
    }
    return os;
}

template<typename T>
size_t LiteMatrix<T>::rowCount() const
{
    return m_rows;
}

template<typename T>
size_t LiteMatrix<T>::colCount() const
{
    return m_cols;
}

template<typename T>
bool LiteMatrix<T>::isAddSubLegal(const LiteMatrix& rhs) const
{
    return ((m_rows == rhs.m_rows) && (m_cols == rhs.m_cols));
}

template<typename T>
bool LiteMatrix<T>::isMultLegal(const LiteMatrix& rhs) const
{
    return (m_cols == rhs.m_rows);
}

template<typename T>
void LiteMatrix<T>::setMatrix(const std::vector<T>& val)
{
    std::copy(val.begin(), val.end(), m_mat.begin());
    return;
}

template<typename T>
void LiteMatrix<T>::setElement(const size_t row, const size_t col, const T& val)
{
    m_mat.at(row * m_rows + col) = val;
    return;
}

template<typename T>
void LiteMatrix<T>::setRow(const size_t row, const std::vector<T> val)
{
    if(row >= m_rows)
        throw std::range_error("Matrix index is out of range\n");
    if (val.size() > m_cols)
    {
        throw std::range_error("Data size is too large\n");
    }

    std::copy(val.begin(), val.end(), m_mat.begin() + row * m_cols);
}

template<typename T>
void LiteMatrix<T>::setCol(const size_t col, const std::vector<T> val)
{
    if (col >= m_cols)
        throw std::range_error("Matrix index is out of range\n");
    if (val.size() > m_rows)
    {
        throw std::range_error("Data size is too large\n");
    }

    for (size_t i = 0; i != val.size(); i++) 
    {
        m_mat[col + i * m_rows] = val[i];
    }
}

template<typename T>
T LiteMatrix<T>::getElement(const size_t row, const size_t col) const
{
    return m_mat.at(row * m_rows + col);
}

template<typename T>
LiteMatrix<T>& LiteMatrix<T>::transpose()
{
    // TODO: insert return statement here
    if (m_cols != 1 && m_rows != 1)
    {
        decltype(m_cols) colStart = 0;
        for (size_t i = 0; i < m_rows; ++i)
        {
            for (size_t j = colStart; j < m_cols; ++j)
            {
                std::iter_swap(m_mat.begin() + (i * m_cols + j),
                    m_mat.begin() + (j * m_rows + i));
            }
            ++colStart;
        }
    }

    std::swap(m_rows, m_cols);

    return *this;
}

main.cpp

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

int main()
{
    LiteMatrix<double> m1(2, 2);
    std::cout << "Row Count: " << m1.rowCount() << std::endl;
    std::cout << "Column Count: " << m1.colCount() << std::endl;

    LiteMatrix<double> m2(2, 2);
    std::cout << m1.ones();
    std::cout << m2.ones();

    LiteMatrix<double> m4(2, 2, std::vector<double>{1, 2, 3, 4});
    LiteMatrix<double> m5(2, 1);
    m5.setMatrix(std::vector<double> {7.5, 10.8});

    LiteMatrix<double> m6(3, 3, std::vector<double>{1, 2, 3, 4, 5, 6, 7, 8, 9});
    std::cout << "m6\n";
    std::cout << m6;
    std::cout << m6.transpose();

    LiteMatrix<double> m7(3, 1, std::vector<double>{1, 2, 3});
    std::cout << "m7\n";
    std::cout << m7;
    std::cout << m7.transpose();

    m1.setElement(0, 0, 19.0);
    std::cout << m1.getElement(0, 0);

    std::cout << "Is Addition Legal: " << m1.isAddSubLegal(m2) << std::endl;

    m1 += m1;
    std::cout << "m1 + m2\n" << m1 + m2;
    std::cout << "m2 - m2\n" << m2 - m2;
    std::cout << "m1 * m2\n" << m1 * m2;

    std::cout << "m1 != m2: " << (m1 != m2) << std::endl;
    std::cout << "m1 == m2: " << (m1 == m2) << std::endl;

    m1.ones();

    std::cout << "m1 != m2: " << (m1 != m2) << std::endl;
    std::cout << "m1 == m2: " << (m1 == m2) << std::endl;

    LiteMatrix<double> m3(10, 10);
    m3.ones();

    std::cout << "Is Addition Legal: " << m1.isAddSubLegal(m3) << std::endl;

    m3.setRow(0, std::vector<double> {22, 33, 44, 55});
    m3.setCol(9, std::vector<double> {66, 77, 88, 99});
    std::cout << m3;
    if(m1.isAddSubLegal(m3))
        m3 += m1;

    return 0;
}
\$\endgroup\$
4
\$\begingroup\$

OverView

I understand that it is common to use rows and cols in the constructor. BUT if you use it as part of the type information you can do some compile time checks that prevents illegal expressions.

For example: In multiplications. You can check that the size of the matrices are correct for the multiplication at compile time.

The down size is that you can not have dynamically sized matrices. So it may not be appropriate for your use case.

Its a matrix why do you not support standard matrix access operations?

LiteMatrix<int>   x(15,5);

std::cout << x[2][3] << "\n";
// Much nicer than 
std::cout << x.getElement(2, 3) << "\n";

CodeReview

Inside the class definition you don't need to specify the <T> everywhere. This is implicit as you are inside the LiteMatrix definition.

template<typename T>
class LiteMatrix
{
    // STUFF
    LiteMatrix operator+(const LiteMatrix<T> &rhs);
    LiteMatrix& operator+=(const LiteMatrix<T>& rhs);

    // STUFF
    template<typename T>
    friend std::ostream& operator<<(std::ostream& os, const LiteMatrix<T>& rhs);
    // STUFF
}

This could simply be:

template<typename T>
class LiteMatrix
{
    // STUFF
    LiteMatrix operator+(const LiteMatrix& rhs);
    LiteMatrix& operator+=(const LiteMatrix& rhs);

    // STUFF
    friend std::ostream& operator<<(std::ostream& os, const LiteMatrix& rhs);
    // STUFF
}

You have a copy object.

    void setMatrix(const std::vector<T>& val);
    void setElement(const size_t row, const size_t col, const T& val);

You might as well have also have a move version!

    void setMatrix(std::vector<T>&& val);
    void setElement(const size_t row, const size_t col, T&& val);

You forgot the reference here:

    // Passing by value is going to cause a copy of the array
    void setRow(const size_t row, const std::vector<T> val);
    void setCol(const size_t col, const std::vector<T> val);

When getting a value return by const reference to avoid an unnecessary copy.

    T getElement(const size_t row, const size_t col) const;

I would also have a non cost version of this that returns a reference to the internal value. That way you can allow updates in a more normal matrix like way.

Why not initialize the vector in the initializer list?

template<typename T>
LiteMatrix<T>::LiteMatrix(const size_t & rows, const size_t & cols)
    : m_rows(rows), m_cols(cols)
{
    m_mat = std::vector<T>(rows * cols);
}

I would have just done:

template<typename T>
LiteMatrix<T>::LiteMatrix(const size_t & rows, const size_t & cols)
    : m_rows(rows)
    , m_cols(cols)
    , m_mat(rows * cols)
{}

Simply return the value of the expression.

template<typename T>
LiteMatrix<T> LiteMatrix<T>::operator+(const LiteMatrix& rhs)
{
    // Simpler to write as:
    return LiteMatrix(*this) += rhs;
}

Yes this works:

template<typename T>
T& LiteMatrix<T>::operator()(const size_t& rIndex, const size_t& cIndex)
{
    return m_mat[rIndex * m_cols + cIndex];
}

But much more traditional to use operator[] on matrix objects. see: https://stackoverflow.com/a/1971207/14065

OK. I see what you are doing here:

template<typename T>
bool LiteMatrix<T>::operator!=(const LiteMatrix& rhs) const
{
    // STUFF
            isNotEqual = std::numeric_limits<T>::epsilon() <= 
                std::abs(m_mat[i * m_cols + j] - rhs.m_mat[i * m_cols + j]);
    // STUFF
}

This is correct. But personally I think I may have gone with some form of type specialization. That would have simply done the test on integer numbers and used epsilon on floating point numbers.

So many lines for simply returning the value. I would have had these as single liners inside the class definition.

template<typename T>
size_t LiteMatrix<T>::rowCount() const
{
    return m_rows;
}

template<typename T>
size_t LiteMatrix<T>::colCount() const
{
    return m_cols;
}
\$\endgroup\$
  • \$\begingroup\$ The comment of removing the <T> in the friend operator doesn't work. It needs to stay in there. When I wrote the code I referenced this and I couldn't get it to work otherwise, stackoverflow.com/a/4660153/10824002. Your post has been very helpful. \$\endgroup\$ – Eddie C. Jan 7 at 4:47
  • \$\begingroup\$ @EddieC.Actually very sure that's not the case: cpp.sh/4r3wj The article you link shows a situation where operator<< is defined outside the class. In that situation you will need the tempalte stuff. \$\endgroup\$ – Martin York Jan 7 at 5:33
  • \$\begingroup\$ I finally had a chance to try it again. if I don't include the <T> with operator<< I get a compiler error of cannot access private member in class LiteMatrix<double>. When I have the <T> the error goes away and the code works as expected. \$\endgroup\$ – Eddie C. Jan 8 at 5:18
  • \$\begingroup\$ @EddieC. I know that it is not necessary. Could you put your code in a gist so I can see the exact code you are talking about. \$\endgroup\$ – Martin York Jan 8 at 6:16
  • \$\begingroup\$ I put what I tried on gist. Sorry for the delay this is the time when i get on. gist.github.com/CodingEddie/49edfad9efca80897fb38d954091f425 \$\endgroup\$ – Eddie C. Jan 9 at 5:29
0
\$\begingroup\$

In the constructor that take a vector, you might want to validate the size and raise an exception if it does not match the expected size.

Since you are using operator() to update a value, why not replace getElement by a const version of operator()? That way, you won't have to use different code for read and for write access.

isAddSubLegal and isMultLegal should probably be private as they are not intended for public use.

I am not sure if operator== and operator!= should be declared. The fact that an epsilon is used for comparison is a red flag. If you need to compare with some tolerance, it might be preferable to use a function that take an additional parameter for the tolerance to use. For an internal library, I would not make such function available unless I actually need it.

\$\endgroup\$

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.