matrix class and overloaded operators

[elshrew]

I'm building a matrix class and got stuck trying to provide the functionality to access and define subsets of the matrix. For example, changing the fifth row of a 10x12 matrix. Although I have code to access a single element (for example A(i,j) = 5) I am looking for a way to replace part of a matrix with another matrix, for example A(rows 2 through 3 and columns 4 through 6) = B. I have achieved the reciprocal (for example B = A(rows 2 through 3 and columns 4 through 6)) however, I get the warning: reference to local variable returned. So I guess I am wasting memory and am also looking for suggestions to avoid this. It's strange because I use the same technique (creating a new instance of the class within a class function) to perform matrix addition but do not get a warning.

To reiterate, using the syntax described by the code below, I can achieve this:

A = B(2,4,2,3);

and need a way to do this:

B(2,4,2,3) = A;

Here I've only included relevant code to explain the problem:

class matrix
{
public:
matrix(int rows = 0, int columns = 0);
matrix &operator=(const matrix &A);
double &operator()(int i, int j);
matrix &operator()(int rowStart, int rowEnd, int colStart, int colEnd);

private:
double *p; // Address of data.
int m, n; // m x n matrix
};

inline matrix& matrix::operator=(const matrix &A)
{
p = A.p;
m = A.m;
n = A.n;
return *this;
}

matrix::matrix(int rows, int columns)
{
m = rows;
n = columns;
p = new double[m*n];
}

double &matrix::operator()(int i, int j)
{
return p[i - 1 + m * (j-1)];
}

matrix &matrix::operator()(int rowStart, int rowEnd, int colStart, int colEnd)
{
int rows = rowEnd - rowStart + 1;
int columns = colEnd - colStart + 1;
matrix subset(rows, columns); //this is where i get the warning
int element = 0;
for (int j = colStart; j <= colEnd; ++j) {
for (int i = rowStart; i <= rowEnd; ++i) {
subset.p[element] = p[i - 1 + m * (j-1)];
element++;
}
}
return subset;
}

[jsmith]

Yes, because subset is a local variable that gets destroyed when the function returns. But you are returning a reference to it (which is very similar to a pointer). So you are returning essentially a reference to unallocated memory.

operator() needs to return a matrix, not a reference to a matrix.

[elshrew]

yes you are right. Considering this example:

A = B(2,4,2,3);

The only problem with this approach is that if A previously existed, the memory originally allocated to it is inaccessible. Anyway, I am really stuck on the other problem.. how to make this work:

B(2,4,2,3) = A;

[jsmith]

To do that, you need proxy objects. B(2,4,2,3) has to return an object of a different type that, when written to, writes back to the original matrix. Consider the following stupid example that demonstrates the idea:

class A_Proxy {
   public:
       // Note you have to put some of this in a .cpp file in order to compile
       A_Proxy( A& obj ) : a_instance( obj ) {}

       A_Proxy& operator=( string val ) {
            a_instance.setValue( val );
            return *this;
       }

       A& a_instance;
};

class A {
   public:
       A_Proxy getValue() const {
            return A_Proxy( *this );
       }

       void setValue( string s ) {
          str = s;
       }

   private:
       string str;
};

[elshrew]

cool, that's a good idea. I'm really new to c so I searched for some other examples of using proxy classes. However, I'm still running into errors trying to implement the above example. How can you mention the proxy class in original class header and vice versa? Is the single colon on line 4 a typo? Although I understand the idea, maybe the syntax in the above example is too shorthand for me.

[jsmith]

In A_Proxy.h:

// Forward declares A:
class A;

class A_Proxy {
  public:
     A_Proxy( A& obj ) : a_instance( obj ) {}

     A_Proxy& operator=( string val );  // Implementation must be in A_Proxy.cpp

  private:
     A& a_instance;
};

The single colon is not a typo. It denotes an initializer list. It is also the ONLY way to initialize a_instance in the above class.

[elshrew]

ok cool - thanks a bundle I learned a lot and got this working, well half of it. Using the proxy class, I got this happening:

B(rowStart,rowEnd,colStart,colEnd) = A;

but now for some reason (???) this does not work as it used to:

B = A(rowStart,rowEnd,colStart,colEnd)

Here is the code (relevant bits) with a simple example that demonstrates the issue:

matrix.h

#include "matrixProxy.h"

#ifndef matrix_H
#define matrix_H

class matrix {
	
public:
	matrix(int rows = 0, int columns = 0);
	double& operator()(int i, int j); //overload function call operator
	matrixProxy operator()(int rowStart, int rowEnd, int colStart, int colEnd);
	matrix& operator=(const matrix &A); //overload assignment operator
	matrix operator=(const matrixProxy &A); //for B = A(rS,rE,cS,cE)
	void replace(matrix R, int rowStart, int rowEnd, int colStart, int colEnd);
	void display(void) const;
	
private:
	double *p;			// Address of data.
	int m, n;			// m x n matrix
};

inline matrix& matrix::operator=(const matrix &A)
{
	p = A.p;
	m = A.m;
	n = A.n;
	return *this;
}

#endif  //#ifndef matrix_H

matrix.cpp

#include "matrix.h"
#include "gaus.h" 
#include <iostream>

matrix::matrix(int rows, int columns)
{
	m = rows;
	n = columns;
	p = new double[m*n];
}

double &matrix::operator()(int i, int j)
{
	return p[i - 1 + m * (j-1)];
}

matrixProxy matrix::operator()(int rowStart, int rowEnd, int colStart, int colEnd)
{
	// create proxyMatrix, the subset of matrix()  
	matrixProxy MP = matrixProxy( *this, rowStart, rowEnd, colStart, colEnd);
	return MP; //subset;
}

matrix matrix::operator=(const matrixProxy &A)
{
	// return new matrix that is subset of matrixProxy &A
	int rows = A.rowEnd - A.rowStart + 1; //matrix is a friend of matrixProxy
	int columns = A.colEnd - A.colStart + 1; 
	matrix subset(rows, columns);
	int element = 0;
	for (int j = A.colStart; j <= A.colEnd; ++j) {
		for (int i = A.rowStart; i <= A.rowEnd; ++i) {
			subset.p[element] = A.matrix_instance.p[i - 1 + rows * (j-1)];
			element++;
		}
	}
	
	std::cout << "subset = \n" ;
	subset.display();
		
	return subset;
}

void matrix::replace(matrix R, int rowStart, int rowEnd, int colStart, int colEnd) {
	//replace rowStart:rowEnd, colStart:colEnd with matrix R
	//TODO: need error checking to ensure size(R) matches row and column width
	
	int element = 0;
	for (int j = colStart; j <= colEnd; ++j) {
		for (int i = rowStart; i <= rowEnd; ++i) {
			p[i - 1 + m * (j-1)] = R.p[element];
			element++;
		}
	}
}

double* matrix::address(void) const
{
	return p;
}

void matrix::display(void) const
{
	for (int i = 1; i <= m; ++i) {			
		std::cout << "\n";
		for (int j = 1; j <= n; ++j) {
			std::cout << p[i - 1 + m * (j-1)] << "\t"; 
			}
		}
	std::cout << "\n\n";
}

matrixProxy.h

#ifndef matrixProxy_H
#define matrixProxy_H

class matrix;

class matrixProxy {

	friend class matrix;
	
	public:
		matrixProxy( matrix& obj, int rS, int rE, int cS, int cE ) : matrix_instance(obj), rowStart(rS), rowEnd(rE),  colStart(cS), colEnd(cE) {} //initializer list
		matrixProxy& operator=(matrix R);
		matrix& matrix_instance;

	private:
		int rowStart, rowEnd, colStart, colEnd;
};


#endif //#ifndef matrixProxy_H 

matrixProxy.cpp

#include "matrix.h"
#include "matrixProxy.h"

matrixProxy& matrixProxy::operator=(matrix R) {
	matrix_instance.replace(R, rowStart, rowEnd, colStart, colEnd);
    return *this;
}

test.cpp

#include "matrix.h"
#include <cmath>
#include <iostream>
using std::cout;


int main()
{
	int m = 5;		//rows
	int n = 3;		//cols
	matrix A(m,n);
	matrix B(m,2);
	
	for (int i = 1; i <= m; ++i) {
		for (int j = 1; j <= n; ++j) {
		A(i,j) = 1.0 * i * j; 
		}
	}
	for (int i = 1; i <= m; ++i) {
		for (int j = 1; j <= 2; ++j) {
		B(i,j) = 100 * i * j; 
		}
	}
	
	cout << "A ="; A.display();
	cout << "A.address = " << A.address() << "\n\n";
	cout << "B ="; B.display();
	cout << "B.address = " << B.address() << "\n\n";
	
	A(1,5,2,3) = B;
	B = A(1,5,1,2); 
	
	cout << "A ="; A.display();
	cout << "A.address = " << A.address() << "\n\n";
	cout << "B ="; B.display();
	cout << "B.address = " << B.address() << "\n\n";

return 0;
}

[jsmith]

Well,

operator=( const matrixproxy& )

really needs affect the instance on which operator= was called, and should therefore return *this, not a copy of a temporary.

[elshrew]

wow. it finally works. thanks much jsmith. For anyone interested here is the correct function:

matrix& matrix::operator=(const matrixProxy& A)
{
	m = A.rowEnd - A.rowStart + 1; 
	n = A.colEnd - A.colStart + 1; 
	int element = 0;
	for (int j = A.colStart; j <= A.colEnd; ++j) {
		for (int i = A.rowStart; i <= A.rowEnd; ++i) {
			p[element] = A.matrix_instance.p[i - 1 + m * (j-1)];
			element++;
		}
	}
return *this;
}