Self operator override - Return by reference or return by copy

Question

Consider the following code?

I was wondering, if I change it from (Function body still the same)

error_code& operator|=(const error_code &e)

to

error_code operator|=(const error_code &e)

Is there any potential bug that might possible occur? The only difference I see is, it will perform an extra copy operation, other than that, no big deal.

So, should I just stick to return by reference, or it doesn't matter?

class error_code {
public:
 error_code() : hi(0), lo(0) {}
 error_code(__int64 lo) : hi(0), lo(lo) {}
 error_code(__int64 hi, __int64 lo) : hi(hi), lo(lo) {}

 // How about return by copy?
 error_code& operator|=(const error_code &e) {
  this->hi |= e.hi;
  this->lo |= e.lo;
  return *this;
 }

 __int64 hi;
 __int64 lo;
};

error_code operator|(const error_code& e0, const error_code& e1) {
 return error_code(e0.hi | e1.hi, e0.lo | e1.lo); 
}

int main() {
 error_code e0(1);
 error_code e1(2);
 e0 |= e1;
}

What is the reason for wanting to change that? – jalf Jan 10 '11 at 09:55 — jalf, Jan 10 '11 at 09:55

score 13 · Accepted Answer · answered Jan 10 '11 at 09:29

13

Is there any potential bug that might possible occur?

Yes. This won't work as expected anymore:

(ec |= x) = y;

Now, this is a silly piece of code, no doubt about that, but

it does work for other types and
there might be other cases where this difference matters.

For example, if your class error_code will have member functions modifying the object they are invoked for, then these will modify a temporary object if you return by copy instead of reference:

(ec |= x).normalize();  // whatever "normalizing" error code means...

answered Jan 10 '11 at 09:29

sbi

219,715
46
258
445

+1 on this. Yan, try to be consistent with the operators' behaviour for the other types. – Simone Jan 10 '11 at 09:32
so, return by reference is the only "zen" of this kind of situation? – Cheok Yan Cheng Jan 10 '11 at 09:36
2

@Yan: A golden rule of operator overloading: __If in doubt, do as the ints do.__ For `int`, `(a|=b)=c` works. (BTW, here's a shameless plug: http://stackoverflow.com/questions/4421706/operator-overloading.) – sbi Jan 10 '11 at 09:42
1

@Yan: yes, you captured it perfectly with the keyword `zen`. It's not mandatory, but unless you have very good reasons (writing a DSEL is usually taken as one, at least by the authors) try not to break people assumptions. – Matthieu M. Jan 10 '11 at 09:42
@sbi `(ec |= x).normalize();` This should be illegal if returning a temporary unless `normalize()` is `const`. – RedX Jan 10 '11 at 09:56
@RedX: You can invoke non-`const` member functions on temporary objects of user-defined types. – sbi Jan 10 '11 at 09:57
1

@RedX: Temporaries are perfectly mutable, you just can't refer to them with mutable references, which is just as stupid as it sounds, and you can take const references to them, but you can't take their address. – Puppy Jan 10 '11 at 10:34

score 1 · Answer 2 · answered Jan 10 '11 at 10:17

It would mean that anything you then do with the returned object would not affect the original object any more. And the other way round - the returned object would never reflect changes done to the original object. This is a typical issue of value vs identity. You are keeping a copy of the object's value, but you're losing its identity.

Self operator override - Return by reference or return by copy

2 Answers2