How to use a static member function to create a matrix which can then be printed using operator overloading?

Question

Using a constructor and operator overloading works as follows with my aim to create a 2x4 matrix of zeros:

Matrix::Matrix(const int noOfRowss, const int noOfCols){

this->noOfRows=noOfRowss;
this->noOfColums=noOfCols;

data= new double[noOfRows*noOfColumns];

    for(int i=0; i< noOfRows; i++){
        for(int j=0; j<noOfColumns; j++){
           int index = i*noOfColumns + j;
           data[index]=0;
        }
    }
}

std::ostream& operator<<(std::ostream& output, const Matrix& rhs){
    for(int i=0; i< rhs.noOfRows; i++){
        for(int j=0; j<rhs.noOfColumns; j++){
            int index = i*rhs.noOfColumns + j;
            output<<rhs.data[index]<< "\t";
        }
        output<<std::endl;
    }
    return output;
}

However when I try to use a static member function, I'm getting a segmentation fault for the following code (see below for implementation in test file):

Matrix Matrix::Zeros(const int noOfRows, const int noOfCols){
    Matrix out;
    for(int i=0; i< noOfRows; i++){
        for(int j=0; j<noOfCols; j++){
           int index = i*noOfCols + j;
           out.data[index]=0;
        }
    }
}

I'm unsure if I'm implementing the static member function correctly, my issue is that in my header function I need to use the following variables:

int noOfRows;
int noOfColumns;
double *data;
int GetIndex(const int rowIdx, const int columnIdx) const;

And in my test file, I want to implement this static member function as follows:

Matrix matrix = Matrix::Zeros(2,4);
cout<<matrix<<endl;

The reason I need to keep the data variable is so it can be used in the operator<< overloading function as it worked before for the constructor. However, having tried several different variations within my static member function I'm not having much luck with storing my matrix in the data variable as easily as before. Does anybody have any suggestions?

score 2 · Answer 1 · edited May 23 '17 at 11:44

So, I see that your static function apparently does this, first.

Matrix output;

However, the constructor code you showed takes two parameters, the number of rows and columns.

From this, I must conclude that you must also have a default constructor that likely constructs an empty matrix, with an empty data vector.

for(int i=0; i< noOfRows; i++){
    for(int j=0; j<noOfCols; j++){
       int index = i*noOfCols + j;
       output.data[index]=0;
    }
}

And then, an attempt here to initialize the contents of the default-constructed matrix, without a validly initialized data member.

This is not going to end well...

P.S., you might want to read this, too: RAII. I suspect that your class will also have related problems in this area, as well. Instead of using a double *data member, a std::vector<double> will work better, and most likely avoid a bunch of pitfalls in this area.

jpo38 · Accepted Answer · 2016-01-05T14:06:45.753

You need to psecify the size of the out object, like that (else, you're filling out.data out of bounds):

Matrix Matrix::Zeros(const int noOfRows, const int noOfCols){
    Matrix out(noOfRows*noOfCols); // changed here
    for(int i=0; i< noOfRows; i++){
        for(int j=0; j<noOfCols; j++){
           int index = i*noOfCols + j;
           out.data[index]=0;
        }
    }
}

If doing that, it will be safer to, at least, follow the rule of three, because using Matrix::Zeros will most likely do construction by copy or assignment by copy in your code (leading to more segfault if not defined correctly, compiler with use default implementation leading to double*pointers being messed up). As pointed by Sam Varshavchik, using std::vector<double> rather than double* will make those default implementations works well in your case.

But, for the specific problem you are trying to address, I would recommend that you simply modify your constructor to make it possible to decide the default matrix content, it may be easier:

Matrix::Matrix(const int noOfRowss, const int noOfCols, double defaultValue)
{
    this->noOfRows=noOfRowss;
    this->noOfColums=noOfCols;

    data= new double[noOfRows*noOfColumns];

    for(int i=0; i< noOfRows; i++){
        for(int j=0; j<noOfColumns; j++){
           int index = i*noOfColumns + j;
           data[index]=defaultValue;
        }
    }
}

In the header file, you can have:

class Matrix
{
public:
     Matrix(const int noOfRowss, const int noOfCols, double defaultValue = 0);
     ...
};

Then:

std::cout << Matrix(4,2) << std::endl; // displays a 4x2 matrix filled with zeros
std::cout << Matrix(4,2,1) << std::endl; // displays a 4x2 matrix filled with ones

I have implemented it in a similar way to how you have written here. Although `Matrix Matrix::Ones(const int noOfRows, const int noOfCols){ Matrix outO(noOfRows, noOfCols, 1); return outO; }` This uses my constructor and then I can implement my code in my original post to use operator << overloading — MachoSkinny, Jan 05 '16 at 23:00
Good. Don't forget the rule of three, to prevent future crashs when manipulating your `Matrix` class. — jpo38, Jan 06 '16 at 07:33

How to use a static member function to create a matrix which can then be printed using operator overloading?

2 Answers2