Function to overload >> to input a matrix (class) isn't working

Question

So, I've written the class, and it looks a bit like this:

class matrix
{
    // Friends
    friend ostream & operator << (ostream &os, const matrix &mat);
    friend istream & operator >> (istream &is, matrix &mat); 
    private:
        double *mdata;
        int rows, columns, size;

And then later on I've written:

// Assignment operator
    public:
        matrix & operator=(const matrix &m)  {
            if(&m == this) {
                return *this; // no self assignment
            }


    // First delete this object's array
        delete[] mdata;
        columns=0;
        rows=0;
        mdata=0;
        int size=0;
        // Now copy size and declare new array
        size=m.getcols()*m.getrows();
        if(size>0) {
            mdata=new double[size];
            // Copy values into new array
            for(int i=0;i<size;i++) {
                mdata[i] = m.mdata[i];
            }
        }
        columns = m.getcols();
        rows = m.getrows();
        return *this; // Special pointer
    }

And I have this outside the class:

ostream & operator << (ostream &os, const matrix &mat) {
    // Code goes here
    os << "\n";
    int j = 1;
    for (int i=0; i < mat.size; i++) {
        os << mat.mdata[i] << " ";
        if (i+1 == j*mat.getcols()) {
            os << "\n";
            j = j + 1;
        }
    }
    os << "\n";
    os << "Wolfram|Alpha code:\n[[";
    j = 1;
    for (int i=0; i < mat.size; i++) {
        os << mat.mdata[i];
        if (i+1 != j*mat.getcols()){
            os << ",";
        }
        if (i+1 == j*mat.getcols()) {
            if (i+1 != mat.size) {
                os << "],[";
            }
            else {
                os << "]";
            }
            j = j + 1;
        }
    }
    os << "] \n";
    return os;
}

istream & operator >> (istream &is, matrix &mat) { 
    is >> mat.rows >> mat.columns;
    int size(mat.rows*mat.columns);
    if(size>0) {
        cout << "Enter " << size << " values (top row, second row... last row - left to right)" << endl;
        mat.mdata=new double[size];
        // Copy values into new array
        for(int i=0;i<size;i++) {
            is >> mat.mdata[i];
        }
    }
    return is;
}

However when running the code (in main):

cout << "Enter the rows and columns of a custom Matrix, row then column:" << endl;
matrix custom;
cin >> custom;
cout << custom;
cout << custom.getrows() << endl;

I don't get any values printed out...

Enter the rows and columns of a custom Matrix, row then column:
Default matrix constructor called
2
2
Enter 4 values (top row, second row... last row - left to right)
1 2 3 4


Wolfram|Alpha code:
[[] 
2
Destructor called

Any ideas about what I'm doing wrong? Full code here

EDIT: Forgot to say, I included the assignment operator because that has the same (or similar) problem.

Answer 1

You never update mat.size in your operator>> overload, so although you read 4 values in, the matrix thinks it is empty and prints nothing out.

Also, very importantly, if the matrix passed to the operator>> overload already has data then you will leak memory, because you do not free that data and you assign a new pointer to mat.mdata

You could do something like this instead:

istream & operator >> (istream &is, matrix &mat) { 
    int rows, columns;
    is >> rows >> columns;
    if (!is)
        return is;  // couldn't read rows and cols
    matrix tmp(rows, columns);
    if(tmp.size>0) {
        cout << "Enter " << tmp.size << " values (top row, second row... last row - left to right)" << endl;

N.B. do not allocate a new array here, that was done in the constructor above.

       // Copy values into new array
        for(int i=0;i<tmp.size;i++) {
            is >> tmp.mdata[i];
        }
    }
    if (is)   // read all values successfully
        mat = tmp;

This requires that your assignment operator works correctly, so it's a good time to ensure that! Another option would be to swap:

        mat.swap(tmp);

This requires a correct matrix::swap(matrix&) member function, which is a useful thing to have for many reasons.

    return is;
}

Notice I put in error-checking to ensure you actually read the expected data from the stream.

The bug in your assignment operator is here:

        int size=0;
        // Now copy size and declare new array
        size=m.getcols()*m.getrows();

You declare a new local variable called size, and update that variable. That means this->size never gets updated. You do not want to declare a new size, you just want to update the member variable, so change the above to:

        // Now copy size and declare new array
        size=m.getcols()*m.getrows();