Return a reference to local object: operator overloading

[Robertlzw]

Hi, C++ folks,

It is dangerous to return a reference to a local object, such as the following:

class Matrix {
   m[4][4];
public:
   Matrix() { } // ctor 
   ... 
   friend Matrix& operator+(const Matrix& arg1, const Matrix& arg2) {
    Matrix m;
    ....
    return m;  // ERROR: return a reference to local object d
 }
};

I am trying to do it this way (the code works):

const int max_temp_matrix = 7;
Matrix& get_temp_matrix() {							
	static int nbuf = 0;
	static Matrix buf[max_temp_size];
	if (max_temp_size == nbuf) nbuf = 0;	
	return buf[nbuf++];
}
Matrix& operator+(const Matrix& arg1, const Matrix& arg2) {
	Matrix& temp = get_temp_matrix();
	for (int i = 0; i < 4; i++)
			for (int j=0; j < 4; j++)
				temp.m[i][j] = arg1.m[i][j] + arg2.m[i][j];
	return temp;
}

The drawback of the above code is that the plus expression can not involve more than 7 temporaries. The following is an improvement on get_temp_matrix() to remove this restriction using a static vector, but it does not work:

Matrix& get_temp_matrix() {							
	static std::vector<Matrix> vec;
	static Matrix d;
	vec.push_back(d);
	return vec.back();
}

Any suggestions on the improved version of get_temp_matrix()?

[helios]

Instead of this, return a new Matrix.

[Robertlzw]

This code works:

Matrix& get_temp_matrix() {
   static Matrix d;
   return d;
}

but as the program keeps running, there is going to be more and more temporary objects floating around. So the above code just has theoretical rather than practical value. For practical code, the buffer approach on the original post should be preferred.

[Disch]

What helios said.

Using a static object like this is a bad idea. operator + should return a Matrix not a Matrix&.

Matrix operator + (const Matrix& l,const Matrix& r)
{
  Matrix sum(l);
  return sum += r;  // of course for this, you'd have to overload += as well
}

[PanGalactic]

One advantage with Disch's approach is that operator+= will be a public member operator, leaving Matrix with no friends. In C++, that's a good thing.

[jsmith]

@PanGalactic:

Just out of curiosity -- I've wondered this myself -- why would you prefer the operators to be implemented as
member functions as opposed to friend functions (obviously some operators must be members due to return
type)? The boost::operators library is made possible by use of friend functions.

[Disch]

I prefer it for organizational reasons.

If all the operators / member functions are inside the class definition, it's easier to see them all and harder to miss them.

The only operators I don't put as members are the ones that can't be (int + MyClass) -- but even those I usually don't make friends because there's no need (they typically just call other operators/ctors).

[jsmith]

But friend functions and their implementations can still be placed in the class definition.

[PanGalactic]

@jsmith: I have been on a kick to keep my code as loosely coupled as possible. Friends are the second-tightest level of coupling in the C++ language after direct inheritance.

An operator that uses public interfaces is resilient because my public interfaces do not change. (That is, they follow the open/closed principle.) That is likely not to be true for friend functions.

As Disch points out, operator+() can be a non-friend free function because operator+=() is a public member function.

[jsmith]

I'm not disagreeing with you, but here's a thought that comes to mind only because I was implementing a matrix
class for grins the other day.

I have a matrix class that implements the function call operator to access elements. I'd like the operator() to
throw an exception if the cell specified is invalid. But this incurs two additional if() checks every call.

Since I am implementing both the matrix class itself and the operators, I'd like the production version to avoid
the extra checking. If I make operator+ a (non-friend) free function, I have no choice but to either incur the
extra overhead of checks or make another unsafe accessor function public. I don't like the second approach
because it means the user can use that method also. (I'd rather have the program fail in a deterministic
fashion than a random one; this is what exceptions buy me).

What I'd really like to do is make the unchecked accessor function private and have my member functions/
operators call the unchecked ones instead.

[PanGalactic]

At some point you have to trust the users of your class to do the right thing. You need an unchecked accessor. They might too. Make it public. The STL has the same issues -- some containers have both checked and unchecked versions of functions (at() vs. operator[], for example) for this very reason -- efficiency vs. safety. And they leave it to the user to decide which to use.

[moorecm]

Although it may be a pain, could you create an Index class to use as a parameter to operator[]? You could then range check in its constructor and once you have a 'valid' Index object as long as you keep it around it could access elements without checking for each access. I suppose this introduces a number of other challenges, though. For example, how to invalidate them when the bounds change, etc.

[moorecm]

I guess another related idea would be to create another abstraction that controls access to the elements. A range object could range check the low and high indexes and then allow access to any of the included indexes.

[helios]

What would be the advantage over a simple if statement?

[moorecm]

I'm just brainstorming...

[jsmith]

At some point you have to trust the users of your class to do the right thing. You need an unchecked accessor. They might too. Make it public. The STL has the same issues -- some containers have both checked and unchecked versions of functions (at() vs. operator[], for example) for this very reason -- efficiency vs. safety. And they leave it to the user to decide which to use.

Why not a debug version vs. production version of the library? debug version checks, production version doesn't.

I know STL has those issues. But vector<>::operator[], for example was done that way so it was bug-for-bug
compatible with C arrays, and I would guess string::operator[] was the same reasoning (with C strings). And
iterators were created to avoid the problem in the first place.