std::map and operator=

[DBarzo]

Hi,

I've a class A that store objects B in a std::map.
B is composed by other objects members and pointers:

class B
{
 private:
   Obj1  o1;
   Obj2* o2_ptr;
};

class A 
{
  private:
    std::map<std::string, B> Bs;
}

First doubt:
If I don't supply the operator= for class B the compiling fails.
Why it cannot use the default operator=()?

Ok, if I create the operator=, in its body I wanna copy the references of the objects members:

B& B::operator=(const B& b)
{
  B tmp();

  &tmp.o1 = &b.o1;
  tmp.o2_ptr = b.o2_ptr;

  swap(tmp, *this);
  return *this;
}

But the compiler tell me that I cannot convert const to non-const neither with const_cast<>!

Any suggestions?
Daniele.

[jsmith]

Is Obj1 copyable?

Your assignment operator is wrong; line 5 makes no sense. I suspect if you fix it, you will be back to your
compile error.

Can you post the actual compile error?

A correct assignment operator, and one that is exception safe, is:

B& B::operator=( B b )
{
    swap( o1, b.o1 );
    swap( o2_ptr, b.o2_ptr );
    return *this;
}

(Of course it is exception safe only if swap() does not throw. It won't for o2_ptr, but could for o1,
depending upon whether or not Obj1::operator= throws.)

[helios]

Line 3 in the second snippet doesn't declare an object named tmp of type B. It declares a function named tmp that takes no parameters and returns a B.

[DBarzo]

For jsmith:

Thanks for the suggestions.
Unfortunately Obj1 and Obj2 are not copiable, using std::swap I get:

error C2582: 'operator =' function is unavailable in 'Obj1' (VS2008)

I don't understand why before I can compile the code! Why I cannot use the default copy?


For helios:

You're right! It's my typing error! :-)


Thanks to both!
Daniele.

[Robertlzw]

To make the assignment operator more efficient, self assignment cases should be tested before the swap:

B& B::operator=( B& b )
{
    if ( this != &b)
   {
    	swap( o1, b.o1 );
    	swap( o2_ptr, b.o2_ptr );
    }
    return *this;
}

[jsmith]

@Robertlzw:

But your version has the limitation that the rhs of the equal sign cannot be const.

EDIT: And indeed, you actually modify the object on the rhs of the equal sign as well as the left!

@DBarzo:

If Obj1 is not copyable, then B isn't copyable either. If you want to make B copyable/assignable,
then Obj1 has to be copyable/assignable also.

[Robertlzw]

@jsmith:
Yes, you are correct. I forgot what swap() is doing. So the prototype should be:

B& B::operator=( B b );

Should we just swap lhs and rhs object regardless of the possible self assignment case? Thanks.

[jsmith]

Since the function takes its rhs by value, the if() check will never be true, even if
the programmer does

a = a;

Therefore, self assignment is necessarily expensive. On the other hand, it's kind
of silly to write

a = a;

[DBarzo]

Thanks to all for your suggestions.

I changed my implementation.
The problem was on inserting object B into the A std::map with operator[] : Bs[key] = newObjB

I changed the implementation using the insert method:

pair<std::string, B> item(key, newObjB);
Bs.insert(item);

(in this case I don't check the insert return value)

In this manner the B object doesn't need to be copyable.

Cheers,
Daniele.