1
\$\begingroup\$

This is my first matrix class which I updated after already asking here already.

The updated code is below.

  • Are there any suggestions regarding member function parameters?
  • Is there a convenient way to get a single function that allows to set/get with a single function?

I considered returning a reference to the vector data member, but ultimately decided that is a terrible idea.

There are also a driver and a data file needed.

#Matrix.h

#ifndef __MATRIX_H__
#define __MATRIX_H__

#include <vector>
#include <iostream>

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

template<typename T>
class Matrix {
private: 
    std::vector<T> data;
    std::size_t rows;
    std::size_t cols;

public:
    Matrix();
    Matrix(const std::vector<T> &, std::size_t rows, std::size_t cols);
    void set(const std::vector<T> &, std::size_t rows, std::size_t cols);
    //Matrix(const Matrix<T>&);

    std::size_t getRows() const;
    std::size_t getCols() const;

    T at(const std::size_t &, const std::size_t &) const; 
    void at(const std::size_t &, const std::size_t &, const T &);

    Matrix row(const std::size_t &) const;
    Matrix col(const std::size_t &) const;

    void print() const;
    friend std::ostream& operator<< <>(std::ostream&, const Matrix<T> &);

    Matrix mult(const Matrix<T> &) const;
    Matrix concat(const Matrix<T> &) const;
    Matrix stack(const Matrix<T> &) const;
    Matrix kronecker(const Matrix<T> &) const;
    T sum() const;

    Matrix operator~() const;                           // transpose
    Matrix operator*(const Matrix<T> &) const;          // Dot product

    //Matrix operator+() const;                         // not useful?

    //elementwise operators 
    //  (No multiplication.  Reserved for dot product)

    Matrix operator-() const;                           // negation of all elements
    Matrix operator+(const Matrix<T> &) const;          //
    Matrix& operator+=(const Matrix<T> &);              //

    Matrix operator-(const Matrix<T> &) const;          //
    Matrix& operator-=(const Matrix<T> &);              //

    Matrix operator/(const Matrix<T> &) const;          //
    Matrix& operator/=(const Matrix<T> &);              //

    //Matrix& operator*=(const Matrix<T> &);            //

    //Scalar Operators


    Matrix operator+(const T &) const;                  //
    Matrix& operator+=(const T &);                      //
    Matrix operator-(const T &) const;                  //
    Matrix& operator-=(const T &);                      //

    Matrix operator*(const T &) const;                  //
    Matrix& operator*=(const T &);                      //
    Matrix operator/(const T &) const;                  //
    Matrix& operator/=(const T &);                      //

    //logical Logical operators

    bool operator==(const Matrix<T> &) const; 
    bool operator!=(const Matrix<T> &) const;           //

};

/** Default Constructor

    creates an empty matrix

*/

template <typename T>
Matrix<T>::Matrix() : 
    data(), rows(0), cols(0) {
}

/** Constructor

    creates the matrix as the following:

    @params elements, - the elements of the matrix in Row-major form
            numRows, - the number of rows in the matrix
            numCols; - the number of coumns in the matrix

*/

template <typename T> 
Matrix<T>::Matrix(const std::vector<T> & elements, std::size_t numRows, std::size_t numCols) :
    data(elements), rows(numRows), cols(numCols) {

        if(data.size() != numRows * numCols) 
            throw std::invalid_argument("matrix dimensions and elments must be equal");

}

/** set

    resets the matrix to the input

    @params elements, - the elements of the matrix in Row-major form
            numRows, - the number of rows in the matrix
            numCols; - the number of coumns in the matrix
    @return void; nothing to return

*/

template <typename T> 
void Matrix<T>::set(const std::vector<T> & elements, std::size_t numRows, std::size_t numCols) {
    rows = numRows;
    cols = numCols;

    data.clear();
    for(unsigned int i = 0; i < elements.size(); i++) {
        data.push_back(elements[i]);
    }
}




/** transpose

    Calculate transpose of matrix

    @return matrix; the transpose of this matrix

*/

template <typename T> 
Matrix<T>  Matrix<T>::operator~() const {
    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(data[(cols*(i%rows)+i/rows)]);
    }
    return Matrix<T>(vec, cols, rows);
}

/** operator(*) dot product 
    lhs * rhs;
    https://en.wikipedia.org/wiki/Matrix_multiplication
    calculate dot product of a matrix

    @params rhs; the second matrix
    @return matrix; the transformed product matrix

*/
template <typename T>
Matrix<T> Matrix<T>::operator*(const Matrix<T> & rhs) const {
    if(cols != rhs.rows) {
        throw std::invalid_argument("can not resolve dot product with operands"); 
    }

    std::vector<T> vec;
    T sum = 0;
    for(unsigned int j = 0; j < rows; j++) {
        for(unsigned int k = 0; k < rhs.cols; k++) {
            for(unsigned int i = 0; i < cols; i++) {
                sum += data[i+j*cols] * rhs.data[k+i*rhs.cols];  
            }
            vec.push_back(sum);
            sum = 0;
        }
    }
    return Matrix(vec,rows,rhs.cols);
}




/** uniary negation operator

    calculate the matrix with all elements negated

    @return matrix; the negated matrix

*/
template <typename T> 
Matrix<T>  Matrix<T>::operator-() const {
    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(-data[i]);
    }   

    return Matrix<T>(vec,rows,cols);
}

/** operator+ (add)
    lhs + rhs;
    elementwise adition of rhs to lhs

    @params rhs; the matrix to add
    @return matrix; the sum

*/

template <typename T>
Matrix<T> Matrix<T>::operator+(const Matrix<T> & rhs) const {

    if(rows != rhs.rows || cols != rhs.cols) {
        throw std::invalid_argument("matrices of unequal dimension");
    }

    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(data[i] + rhs.data[i]);
    }   

    return Matrix<T>(vec,rows,cols);
}

/** operator- (subtract)
    lhs - rhs;
    elementwise subtraction of rhs from lhs

    @params rhs; the matrix to subtract
    @return matrix; the difference

*/

template <typename T>
Matrix<T> Matrix<T>::operator-(const Matrix<T> & rhs) const {
    if(rows != rhs.rows || cols != rhs.cols) {
        throw std::invalid_argument("matrices of unequal dimension");
    }

    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(data[i] - rhs.data[i]);
    }   

    return Matrix<T>(vec,rows,cols);
}

/** operator+= 
    lhs + rhs;
    elementwise adition of rhs to lhs

    @params rhs; the matrix to add
    @return matrix; the reference to this matrix

*/
template <typename T> 
Matrix<T>& Matrix<T>::operator+=(const Matrix<T> & rhs) {
    if(rows != rhs.rows || cols != rhs.cols) {
        throw std::invalid_argument("matrices of unequal dimension");
    }

    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        data[i] += rhs.data[i];
    }   

    return *this;   
}

/** operator-= 
    lhs - rhs;
    elementwise subtraction of rhs from lhs

    @params rhs; the matrix to subtract
    @return matrix; the reference to this matrix

*/
template <typename T> 
Matrix<T>& Matrix<T>::operator-=(const Matrix<T> & rhs) {
    if(rows != rhs.rows || cols != rhs.cols) {
        throw std::invalid_argument("matrices of unequal dimension");
    }

    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        data[i] -= rhs.data[i];
    }   

    return *this;   
}


template <typename T> 
Matrix<T> Matrix<T>::operator/(const Matrix<T> & rhs) const {
    if(rows != rhs.rows || cols != rhs.cols) {
        throw std::invalid_argument("matrices of unequal dimension");
    }

    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(data[i] / rhs.data[i]);
    }   

    return Matrix<T>(vec,rows,cols);
}

template <typename T> 
Matrix<T>& Matrix<T>::operator/=(const Matrix<T> & rhs) {
    if(rows != rhs.rows || cols != rhs.cols) {
        throw std::invalid_argument("matrices of unequal dimension");
    }

    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        data[i] /= rhs.data[i];
    }   

    return *this;   
}




template <typename T>
Matrix<T>  Matrix<T>::operator+(const T & t) const {
    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(data[i] + t);
    }   

    return Matrix<T>(vec,rows,cols);
}

template <typename T>
Matrix<T>& Matrix<T>::operator+=(const T & t) {

    for(unsigned int i = 0; i < data.size(); i++) {
        data[i] += t;
    }   

    return *this;
}

template <typename T>
Matrix<T>  Matrix<T>::operator-(const T & t) const {
    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(data[i] - t);
    }   

    return Matrix<T>(vec,rows,cols);
}

template <typename T>
Matrix<T>& Matrix<T>::operator-=(const T & t) {

    for(unsigned int i = 0; i < data.size(); i++) {
        data[i] -= t;
    }   

    return *this;
}

/** operator* (scalar multiplication)
    M<T> * T;
    calculate scalar product of a matrix

    @params rhs; the scalar;
    @return matrix; the transformed product matrix

*/
template <typename T>
Matrix<T> Matrix<T>::operator*(const T & t) const {
    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(data[i] * t);
    }   

    return Matrix<T>(vec,rows,cols);
}

template <typename T>
Matrix<T>& Matrix<T>::operator*=(const T & t) {

    for(unsigned int i = 0; i < data.size(); i++) {
        data[i] *= t;
    }   

    return *this;
}       

template <typename T>
Matrix<T>  Matrix<T>::operator/(const T & t) const {
    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(data[i] / t);
    }   

    return Matrix<T>(vec,rows,cols);
}                   

template <typename T>
Matrix<T>& Matrix<T>::operator/=(const T & t)  {

    for(unsigned int i = 0; i < data.size(); i++) {
        data[i] /= t;
    }   

    return *this;
}                       



/** operator ==

    elemetnwise comparison of two matrices of equal size 

    @params rhs; the second matrix
    @return bool; true if same size and elements all equal 

*/
template <typename T>
bool Matrix<T>::operator==(const Matrix<T> & rhs) const {

    if(rows != rhs.rows || cols != rhs.cols) {
        return false;
    }

    for(unsigned int i = 0; i < data.size(); i++) {
        if(data[i] != rhs.data[i]) 
            return false;
    }   

    return true;
}

/** operator !=

    elemetnwise comparison of two matrices of equal size 

    @params rhs; the second matrix
    @return bool; false if same size and elements all equal 

*/
template <typename T>
bool Matrix<T>::operator!=(const Matrix<T> & rhs) const {

    if(rows != rhs.rows || cols != rhs.cols) {
        return true;
    }

    for(unsigned int i = 0; i < data.size(); i++) {
        if(data[i] != rhs.data[i]) 
            return true;
    }   

    return false;
}

/** ostream operator

    adds elements to output stream
    formatted 
        e11, e12
        e21, e22

        @params os, rhs; ostream refernece and matrix to output
        @return os, ostream reference

*/

template <typename T>
std::ostream& operator<<(std::ostream& os, const Matrix<T> & rhs) {
    for(unsigned int i = 0; i < rhs.data.size(); i++) {
        os << rhs.data[i] << "  ";
        if((i+1)%rhs.cols == 0) 
            os << std::endl;
    }
    return os;
}

template <typename T>
void Matrix<T>::print() const {
    for(unsigned int i = 0; i < data.size(); i++) {
        std::cout << data[i] << ", ";
        if((i+1) % cols == 0)
            std::cout << std::endl;
    }
}




template <typename T>
T Matrix<T>::sum() const {
    T t = 0;
    for(unsigned int i = 0; i < data.size(); i++) {
        t += data[i];
    }
    return t;
}

/** multiplication (Hardamard Product)
    https://en.wikipedia.org/wiki/Hadamard_product_(matrices)
    calculate elemetnwise product of a matrix

    @params rhs; the second matrix
    @return matrix; the transformed product matrix

*/

template <typename T>
Matrix<T> Matrix<T>::mult(const Matrix<T> & rhs) const {
    if(rows != rhs.rows || cols != rhs.cols) {
        throw std::invalid_argument("matrices of unequal dimension!");
    }

    std::vector<T> vec;
    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(data[i] * rhs.data[i]);
    }   

    return Matrix<T>(vec,rows,cols);
}


/** Concat

    append two matrices of equal row count

    @params rhs; the matrix to concatanate
    @return matrix; the contanated matrix

*/

template <typename T>
Matrix<T> Matrix<T>::concat(const Matrix<T> & rhs) const {

    if(rows != rhs.rows) 
        return Matrix<T>(*this);

    std::vector<T> vec;
    for(unsigned int i = 0; i < rows; i++) {
        for(unsigned int j = 0; j < cols; j++) {
            vec.push_back(data[i*cols + j]);
        }
        for(unsigned int j = 0; j < rhs.cols; j++) {
            vec.push_back(rhs.data[i*rhs.cols + j]);
        }
    }

    return Matrix<T>(vec,rows,cols+rhs.cols);
}

/** stack

    append two matrices of equal column count

    @params rhs; the matrix to stack below 
    @return matrix; the lhs stacked ontop of rhs matrix

*/

template <typename T>
Matrix<T> Matrix<T>::stack(const Matrix<T> & rhs) const {

    if(cols != rhs.cols) 
        return Matrix<T>(*this);

    std::vector<T> vec;

    for(unsigned int i = 0; i < data.size(); i++) {
        vec.push_back(data[i]);
    }
    for(unsigned int i = 0; i < rhs.data.size(); i++) {
        vec.push_back(rhs.data[i]);
    }

    return Matrix<T>(vec,rows+rhs.rows,cols);
}

/** Kronecker
    https://en.wikipedia.org/wiki/Kronecker_product
    calculate kroncker product of two matrices

    @params rhs; the matrix operand
    @return matrix; the Kronecker product matrix

*/

template <typename T>
Matrix<T> Matrix<T>::kronecker(const Matrix<T> & rhs) const {

    std::vector<T> vec;

    for(unsigned int i = 0; i < (rows*cols*rhs.rows*rhs.cols); i++) {
        unsigned int j = (i/rhs.cols)%cols + (i/(cols*rhs.rows*rhs.cols))*cols; //iterate lhs in proper order
        unsigned int k = (i%rhs.cols) + ((i / (cols * rhs.cols))%rhs.rows)*rhs.cols;  //iterate rhs in proper order
        //can use scalar multiplactions, matrix concat and stacking, but this is a single iteration through the vector.
        //Kronecker iterates both matrices in a pattern relative to the large product matrix.
        //std::cout << i << " : " << j << " : " << k << std::endl; 
        //std::cout << i << " : " << j << " : " << k << " : " << l << std::endl;
        vec.push_back(data[j]*rhs.data[k]);
    }

    return Matrix<T>(vec,rows*rhs.rows,cols*rhs.cols);
}

template <typename T>
std::size_t Matrix<T>::getRows() const {
    return rows;
}

template <typename T>
std::size_t Matrix<T>::getCols() const {
    return cols;
}


template <typename T>
T Matrix<T>::at(const std::size_t & row,const std::size_t & col) const {

    if(row > rows || col > cols) {
        throw std::invalid_argument("Indices out of bounds!");
    }

    return data[row*cols+col];

}


template <typename T>
void Matrix<T>::at(const std::size_t & row,const std::size_t & col, const T & t)  {

    if(row > rows || col > cols) {
        throw std::invalid_argument("Indices out of bounds!");
    }

    data[row*cols+col] = t;

}


template <typename T>
Matrix<T> Matrix<T>::row(const std::size_t & r) const {
    if(r > rows) 
        throw std::invalid_argument("indices out of bounds!");

    std::vector<T> subVec;
    for(unsigned int i = r * cols; i < (r+1) * cols; i++) {
        subVec.push_back(data[i]);
    }
    return Matrix<T>(subVec, 1, cols);
}

template <typename T>
Matrix<T> Matrix<T>::col(const std::size_t & c) const {
    if(c > cols) 
        throw std::invalid_argument("indices out of bounds!");

    std::vector<T> subVec;
    for(unsigned int i = c; i < data.size(); i+=cols ) {
        subVec.push_back(data[i]);
    }
    return Matrix<T>(subVec, rows, 1);
}


#endif

Source.cpp

#include <iostream>
#include <vector>
#include "Matrix.h"
#include "NeuralNet.h"
#include <string>
#include <fstream>
#include <sstream>


Matrix<float> loadData(std::string); 
//bool saveData(Matrix, std::string); //Not implemented yet


void testMatrixClass(std::vector<Matrix<float>> &, std::vector<Matrix<int>> &);
bool loadTestData(std::string, std::vector<Matrix<float>> &, std::vector<Matrix<int>> &, std::vector<std::string> &); //for use with my test implementaiton


int main() {

    std::vector<Matrix<float>> fMats;
    std::vector<Matrix<int>> iMats;
    std::vector<std::string> names;
    if (!loadTestData("testData.data", fMats, iMats, names)) {
        std::cout << "Error Loading Data";
        return 0;
    }
    try {
        testMatrixClass(fMats,iMats);
    } catch (std::exception e) {
        std::cerr << e.what() << std::endl;
    }


    return 0;
}








//
Matrix<float> loadData(std::string fileName) {
    //TODO: Implement file loading and data parsing
    std::vector<float> vec;

    std::ifstream inFile(fileName);

    if(!inFile.is_open()) { 
        Matrix<float> matrix;
        return matrix;
    }

    std::string row;
    std::stringstream ss;

    int rowCnt = 0;
    int colCnt = 0;
    while(inFile.good() && std::getline(inFile, row)) {

        ss << row;
        rowCnt++;
        float temp;

        while( ss >> temp ) {
            colCnt++;
            vec.push_back(temp);
        }       
        ss.clear();

    }


    colCnt = colCnt / rowCnt;
    return Matrix<float>(vec,rowCnt,colCnt);

}


//bool saveData(Matrix, std::string) {
//
//  return true;
//}

bool loadTestData(std::string fileName, std::vector<Matrix<float>> & floatMat, std::vector<Matrix<int>> & intMat, std::vector<std::string> & description) {
    std::vector<float> vec;
    std::vector<int> intVec;

    std::ifstream inFile(fileName);

    if(!inFile.is_open()) { 
        return false;
    }

    std::string row;
    std::stringstream ss;

    int rowCnt = 0;
    int colCnt = 0;

    while(inFile.good() && std::getline(inFile, row)) {

        if(row.find('=',0) != std::string::npos) {  
            colCnt = colCnt / rowCnt;
            floatMat.push_back(Matrix<float>(vec,rowCnt,colCnt));
            intMat.push_back(Matrix<int>(intVec,rowCnt,colCnt));
            intVec.clear();
            vec.clear();
            rowCnt = 0;
            colCnt = 0;
        } else if(row.find("#",0) != std::string::npos) {           
            description.push_back(row.substr(1));
        } else {            
            ss << row;
            rowCnt++;
            float temp;
            while( ss >> temp ) {
                colCnt++;
                vec.push_back(temp);
                intVec.push_back((int)temp);
            }       

        }

        ss.clear();
    }

    inFile.close();
    return true;
}
void testMatrixClass(std::vector<Matrix<float>> & fmats, std::vector<Matrix<int>> & imats) {

    std::vector<std::string> floatTest;
    std::vector<bool> floatResults;
    std::vector<std::string> intTest;
    std::vector<bool> intResults;

    floatTest.push_back("Constructor Test");
    bool flag = true;
    for(int i = 0; i < 12; i++) {
        if((i+1) != fmats[0].at( i / fmats[0].getCols(), i % fmats[0].getCols()))   
        flag = false;
    }
    floatResults.push_back(flag);

    floatTest.push_back("Copy Constructor Test");
    floatResults.push_back(Matrix<float>(fmats[0]) == fmats[0]);

    floatTest.push_back("Transpose test");
    floatResults.push_back(fmats[1] == ~fmats[0]);

    floatTest.push_back("Dot Product");
    floatResults.push_back(fmats[2] * fmats[3] == fmats[4]);

    floatTest.push_back("Dot Product");
    floatResults.push_back(~fmats[3] * ~fmats[2] == ~fmats[4]);


    floatTest.push_back("add");
    floatTest.push_back("sub");
    floatTest.push_back("mult");
    floatTest.push_back("div");

    floatResults.push_back(fmats[5] + fmats[6] == fmats[7]);
    floatResults.push_back(fmats[5] - fmats[6] == fmats[8]);
    floatResults.push_back(fmats[5].mult(fmats[6]) == fmats[9]); //hadamard multiplcation (elementwise)
    floatResults.push_back(fmats[5] / fmats[6] == fmats[10]);

    floatTest.push_back("scalar add");
    floatTest.push_back("scalar sub");
    floatTest.push_back("scalar mult");
    floatTest.push_back("scalar div");

    floatResults.push_back(fmats[5] + 2 == fmats[11]);
    floatResults.push_back(fmats[5] - 2 == fmats[12]);
    floatResults.push_back(fmats[5] * 2 == fmats[13]);
    floatResults.push_back(fmats[5] / 2 == fmats[14]);

    floatTest.push_back("== fail test");
    floatTest.push_back("!= test");
    floatTest.push_back("!= fail test");

    floatResults.push_back(!(fmats[0] == fmats[1]));
    floatResults.push_back(fmats[0] != fmats[1]);
    floatResults.push_back(!(fmats[1] != fmats[1]));

    floatTest.push_back("element read");
    floatResults.push_back(fmats[15].at(0,2) == 2 && fmats[15].at(1,0) == 2 && fmats[15].at(3,1) == fmats[15].at(2,3));

    fmats[15].at(0, 2, 0.0f);
    fmats[15].at(1, 0, 0.0f);
    fmats[15].at(3, 1, 0.0f);
    fmats[15].at(2, 3, 0.0f);

    floatTest.push_back("element access");
    floatResults.push_back(fmats[15].at(0,2) == 0);

    floatTest.push_back("element access");
    floatResults.push_back(fmats[16] == fmats[15]);

    floatTest.push_back("negation");
    floatResults.push_back(-fmats[16] == fmats[17]);

    floatTest.push_back("Fun");
    floatResults.push_back(fmats[16] * fmats[18] == fmats[18]);



    floatTest.push_back("+= return ref");
    fmats[5] += fmats[5];
    floatResults.push_back(fmats[13] == fmats[5]);






    intTest.push_back("Constructor Test");
    flag = true;
    for(int i = 0; i < 12; i++) {
        if((i+1) != imats[0].at( i / imats[0].getCols(), i % imats[0].getCols()))
            flag = false;
    }
    intResults.push_back(flag);

    intTest.push_back("Copy Constructor Test");
    intResults.push_back(Matrix<int>(imats[0]) == imats[0]);

    intTest.push_back("Transpose test");
    intResults.push_back(imats[1] == ~imats[0]);

    intTest.push_back("Dot Product");
    intResults.push_back(imats[2] * imats[3] == imats[4]);

    intTest.push_back("Dot Product"); 
    intResults.push_back(~imats[3] * ~imats[2] == ~imats[4]);

    intTest.push_back("Matrix add");
    intTest.push_back("Matrix sub");
    intTest.push_back("Matrix mult (Hadamard)");
    intTest.push_back("Matrix div");

    intResults.push_back(imats[5] + imats[6] == imats[7]);
    intResults.push_back(imats[5] - imats[6] == imats[8]);
    intResults.push_back(imats[5].mult(imats[6]) == imats[9]); //hadamard multiplcation (elementwise)
    intResults.push_back(imats[5] / imats[6] == imats[10]);

    intTest.push_back("scalar add");
    intTest.push_back("scalar sub");
    intTest.push_back("scalar mult");
    intTest.push_back("scalar div");

    intResults.push_back(imats[5] + 2 == imats[11]);
    intResults.push_back(imats[5] - 2 == imats[12]);
    intResults.push_back(imats[5] * 2 == imats[13]);
    intResults.push_back(imats[5] / 2 == imats[14]);

    intTest.push_back("== fail test");
    intTest.push_back("!= test");
    intTest.push_back("!= fail test");

    intResults.push_back(!(imats[0] == imats[1]));
    intResults.push_back(imats[0] != imats[1]);
    intResults.push_back(!(imats[1] != imats[1]));

    intTest.push_back("element read");
    intResults.push_back(imats[15].at(0,2) == 2 && imats[15].at(1,0) == 2 && imats[15].at(3,1) == imats[15].at(2,3));

    imats[15].at(0, 2, 0);
    imats[15].at(1, 0, 0);
    imats[15].at(3, 1, 0);
    imats[15].at(2, 3, 0);

    intTest.push_back("element access");
    intResults.push_back(imats[15].at(0,2) == 0);

    intTest.push_back("element access");
    intResults.push_back(imats[16] == imats[15]);

    intTest.push_back("negation");
    intResults.push_back(-imats[16] == imats[17]);

    intTest.push_back("Fun");
    intResults.push_back(imats[16] * imats[18] == imats[18]);

    intTest.push_back("+= return ref");
    imats[5] += imats[5];
    intResults.push_back(imats[13] == imats[5]);


    std::cout << std::endl << std::string(10,' ') << "Integers" << std::string(30,' ') << "Floats" << std::endl;
    for(unsigned int i = 0; i < intTest.size(); i++) {
        std::cout << intTest[i] << std::string(24 - intTest[i].length(),'_') << ":  ";
        if(intResults[i])
            std::cout << "passed.   ";
        else
            std::cout << "failed.   ";

        std::cout << floatTest[i] << std::string(24 - floatTest[i].length(),'_') << ":  ";
        if(floatResults[i])
            std::cout << "passed." << std::endl;
        else
            std::cout << "failed." << std::endl;
    }
    std::cout << std::endl << "Thanks mang!" << std::endl; 
    return;
}

testData.data

#Con
1 2 3 4
5 6 7 8
9 10 11 12
=
#~Con (transpose)
1 5 9
2 6 10
3 7 11
4 8 12
=
#dLHS
2 1 
1 2
=
#dRHS
1 2 2 1
2 1 1 2
=
#dLHS * dRHS (dot)
4 5 5 4
5 4 4 5
=
#LHS
1 2 3
3 2 1
1 3 1
=
#RHS
1 1 2
2 1 1
1 1 1
=
#LHS + RHS
2 3 5
5 3 2
2 4 2
=
#LHS - RHS
0 1 1
1 1 0
0 2 0
=
#LHS o RHS (Hadamard)
1 2 6
6 2 1
1 3 1
=
#LHS / RHS
1 2 1.5
1.5 2 1
1 3 1
=
#LHS + 2
3 4 5
5 4 3
3 5 3
=
#LHS - 2
-1 0 1
1 0 -1
-1 1 -1
=
#LHS * 2
2 4 6
6 4 2
2 6 2
=
#LHS / 2
0.5 1 1.5
1.5 1 0.5
0.5 1.5 0.5
=
#Element Access
1 0 2 0
2 1 0 2
0 0 1 2
0 2 0 1
=
#ResMat
1 0 0 0
0 1 0 0
0 0 1 0
0 0 0 1
=
#-ResMat
-1 0 0 0
0 -1 0 0
0 0 -1 0
0 0 0 -1
=
#Fun
1 2 3 4
5 6 7 8
9 10 11 12
13 14 15 16
= 
\$\endgroup\$
  • \$\begingroup\$ “I considered returning a reference to the vector data member, but ultimately decided that is a terrible idea.” Why? This is what std::vector does, and many other standard containers. \$\endgroup\$ – Cris Luengo Nov 3 '18 at 20:44
  • \$\begingroup\$ The calling function could hold onto a pointer that gets invalidated? \$\endgroup\$ – Chemistpp Nov 3 '18 at 20:58
  • 2
    \$\begingroup\$ The calling code could also new an object of your class, delete it, and then dereference the pointer. You don’t need to protect against stupidity. std::vector::push_back is documented to invalidate iterators and references to data elements. They’re not going to make iterators inefficient just because someone could use them in the wrong way. You should follow that guiding principle in your code as well, IMO. \$\endgroup\$ – Cris Luengo Nov 3 '18 at 21:34
  • \$\begingroup\$ @CrisLuengo I see your point about the calling code and protecting against their improper use. I thought about this comment for a bit, I don't understand what you mean about "You should follow that guiding principle in your code as well".. I took that as either- stop trying to prevent improper use/error checking so much or implement iterators? Or implement my own matrix container instead of using vectors? Thanks for your note! \$\endgroup\$ – Chemistpp Nov 4 '18 at 13:28
3
\$\begingroup\$

There's a lot of code here to review, so this is likely not going to be a complete review.

Data access

I'll start with one of your questions:

Is there a convenient way to get a single function that allows to set/get with a single function?

As I understand this question (and what I was getting at in the comments) is that you want to get and set elements of the matrix with one function. Currently you have these:

T at(const std::size_t &, const std::size_t &) const;
void at(const std::size_t &, const std::size_t &, const T &);

I would write these instead:

T& at(std::size_t i, std::size_t j);
T at(std::size_t i, std::size_t j) const;

The first, non-const version of this function can be used to both get and set the value:

Matrix<double> A(...);
double a = A.at(2,3);
A.at(2,3) = 3.14159;

The const version will be used for const matrices:

void myFunction(Matrix<double> const& B) {
   double b = B.at(2,3);
   //B.at(2,3) = 3.14159; // does not compile, B is const
}

You currently have a function to completely replace the data (set), but none to retrieve the data. Maybe you were asking about this in your question. Indeed, that is harder to do in a nice way. One approach could be a function gut that moves the array out of your matrix and leaves it in an empty state:

std::vector<T> gut() {
   std::vector<T> out;
   std::swap(out, data);
   rows = cols = 0;
   return out;       
}

Constructors

I would suggest you add a constructor that does not require a std::vector be given:

Matrix(std::size_t rows, std::size_t cols);

Your set method is not much more than a constructor. This call:

Matrix<double> A(...);
std::vector v(...);

A.set(v, ...);

is functionally identical to:

A = Matrix<double>(v, ...);

In the case of very large matrices, the additional allocation might be problematic, I don't know. But the set method potentially does a lot of allocations. You need to add a data.reserve(elements.size()) in there.

Function arguments

Are there any suggestions regarding member function parameters?

You have some places where you take std::size by value, and some places where you take it by const reference. A scalar value like that is never efficient to take by reference. These should all be by value.

Operators

I'm not very excited about the idea to (ab)use ~ as the transpose operator, but I guess it can work. In general, I advice against using operators in a non-standard way, unless in the domain of application the operator would not be surprising. But in this case, ~ being a bit-wise operator that is not often used, it might not hurt a lot. C++ itself abuses << and >> as stream operators, whereas they originally were bitwise shift operators.

You should implement your arithmetic operators in terms of compound assignment operators. For example:

template <typename T>
Matrix<T>& Matrix<T>::operator+=(const Matrix<T> & rhs) {
   if(rows != rhs.rows || cols != rhs.cols) {
      throw std::invalid_argument("matrices of unequal dimension");
   }

   std::vector<T> vec;
   for(unsigned int i = 0; i < data.size(); i++) {
      data[i] += rhs.data[i];
   }

   return *this;
}

could be more simply written as a free function (i.e. not a member function):

Matrix<T>  operator+(Matrix<T> lhs, Matrix<T> const& rhs) {
   lhs += rhs;
   return lhs;
}

Note that we take lhs by copy. This copy will be our new, output matrix. We use += to implement the logic. This avoids code duplication.

Note that in the implementation of +=, and in many other places, you should tell the compiler what size the output data vector will be:

std::vector<T> vec;
for(unsigned int i = 0; i < data.size(); i++) {
   vec.push_back(data[i] + rhs.data[i]);
}

can be more efficiently written as:

std::vector<T> vec;
vec.reserve(data.size());
for(unsigned int i = 0; i < data.size(); i++) {
   vec.push_back(data[i] + rhs.data[i]);
}

or:

std::vector<T> vec(data.size());
for(unsigned int i = 0; i < data.size(); i++) {
   vec[i] = data[i] + rhs.data[i];
}

Other things

In C++, it is considered best practice to put the const modifier after the thing it modifies. These things are the same:

const Type&
Type const&

However, the second form prevents some errors and misunderstandings when Type is an alias. I would advice you get used to writing Matrix const&.

Your Matrix class would be more portable if it were declared inside a namespace. If I write a library using your Matrix class, and someone else writes a different library using a different Matrix class (it's not like "matrix" is not a common name, so this is actually likely!) then these two libraries can never be used together in the same program. Using namespaces this is easily solved.

\$\endgroup\$
  • \$\begingroup\$ Thank you! I really appreciate your time. \$\endgroup\$ – Chemistpp Nov 4 '18 at 14:36

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.