question on the assignment operator

M

ma740988

Consider:

# include <iostream>
# include <algorithm>
# include <vector>
# include <string>
using namespace std;

class msg {
std::string someStr;
public:
msg(const std::string& sStr)
: someStr(sStr) {}
msg( const msg& rhs )
: someStr(rhs.someStr)
{}
void swap( msg& rhs ) {
std::swap( someStr, rhs.someStr );
}
msg& operator=( const msg& rhs) {
msg tmp (rhs);
swap(tmp);
return *this;
}
std::string get_some_str() const { return someStr; }
};

class Y {
typedef std::vector<msg> msgVec;
msgVec msg_vec;
public:
Y(){}
Y( const Y& rhs )
: msg_vec(rhs.msg_vec)
{}
void swap( Y& rhs ) {
std::swap( msg_vec, rhs.msg_vec );
}
Y& operator=( const Y& rhs) {
Y tmp (rhs);
swap(tmp);
return *this;
}
void add(msg& ms)
{ msg_vec.push_back(ms); }

void print()
{
if (msg_vec.size())
std::cout << msg_vec[0].get_some_str() << std::endl; // just for
test
}

};

class Controller {
typedef std::vector<Y> VEC_Y;
VEC_Y vectorOfYs;
public:

Controller() {}
Controller( const Controller& rhs )
: vectorOfYs(rhs.vectorOfYs)
{}

void swap( Controller& rhs ) {
std::swap( vectorOfYs, rhs.vectorOfYs );
}
Controller& operator=( const Controller& rhs) {
Controller tmp (rhs);
swap(tmp);
return *this;
}

void add_y(Y myY)
{ vectorOfYs.push_back(myY);}

void add_msg ( msg& ms )
{
if (vectorOfYs.size())
vectorOfYs[0].add(ms); // brute force for test.
}
void print() {
if (vectorOfYs.size())
vectorOfYs[0].print(); // just experimenting
}
};

int main()
{
Controller ctrl1;
ctrl1.add_y(Y());
ctrl1.add_msg(msg("test"));

Controller ctrl2;
ctrl2 = ctrl1;
ctrl2.print();
}


For assignment between two controller objects, the fact that the class
Controller contains a vector of Y's which in turn contains a vector of
msg's; implies that theres - perhaps a requirement for a copy
constructor in all _three_ classes. Correct?
 
V

Victor Bazarov

ma740988 said:
Consider:

# include <iostream>
# include <algorithm>
# include <vector>
# include <string>
using namespace std;

class msg {
std::string someStr;
public:
msg(const std::string& sStr)
: someStr(sStr) {}
msg( const msg& rhs )
: someStr(rhs.someStr)
{}
void swap( msg& rhs ) {
std::swap( someStr, rhs.someStr );
}
msg& operator=( const msg& rhs) {
msg tmp (rhs);
swap(tmp);
return *this;

Really? No, REALLY? Why not simply

someStr = rhs.someStr;
return *this;

?
}
std::string get_some_str() const { return someStr; }
};

class Y {
typedef std::vector<msg> msgVec;
msgVec msg_vec;
public:
Y(){}
Y( const Y& rhs )
: msg_vec(rhs.msg_vec)
{}
void swap( Y& rhs ) {
std::swap( msg_vec, rhs.msg_vec );
}
Y& operator=( const Y& rhs) {
Y tmp (rhs);
swap(tmp);

Again... WHY? Just assign:

msg_vec = rhs.msg_vec;
return *this;
}
void add(msg& ms)

void add(msg const & ms) // a bit better
{ msg_vec.push_back(ms); }

void print()
{
if (msg_vec.size())
std::cout << msg_vec[0].get_some_str() << std::endl; // just for
test
}

};

class Controller {
typedef std::vector<Y> VEC_Y;
VEC_Y vectorOfYs;
public:

Controller() {}
Controller( const Controller& rhs )
: vectorOfYs(rhs.vectorOfYs)
{}

void swap( Controller& rhs ) {
std::swap( vectorOfYs, rhs.vectorOfYs );
}
Controller& operator=( const Controller& rhs) {
Controller tmp (rhs);
swap(tmp);

return *this;
}

void add_y(Y myY)

void add_y(Y const & myY)
{ vectorOfYs.push_back(myY);}

void add_msg ( msg& ms )
{
if (vectorOfYs.size())

if (!vectorOfYs.empty()) // better
vectorOfYs[0].add(ms); // brute force for test.
}
void print() {
if (vectorOfYs.size())

if (!vectorOfYs.empty()) // better
vectorOfYs[0].print(); // just experimenting
}
};

int main()
{
Controller ctrl1;
ctrl1.add_y(Y());
ctrl1.add_msg(msg("test"));

Controller ctrl2;
ctrl2 = ctrl1;
ctrl2.print();
}


For assignment between two controller objects, the fact that the class
Controller contains a vector of Y's which in turn contains a vector of
msg's; implies that theres - perhaps a requirement for a copy
constructor in all _three_ classes. Correct?

No, not really. Since none of those objects require any special
processing during copy-construction, the compiler-generated one
is just right.

Besides, why are you doing all this nonsense in the assignment
operators? Simply assign the damn members and move on to more
important things...

V
 
G

Guest

Victor Bazarov said:
Really? No, REALLY? Why not simply

someStr = rhs.someStr;
return *this;

Even further, why define operator= at all? Defining operator= unnecessarily
opens the risk of forgetting to assign any members that may be added in the
future. Forgetting to assign the base classes is another problem that I've
lived in the past.

[...]
No, not really. Since none of those objects require any special
processing during copy-construction, the compiler-generated one
is just right.

Agreed... I wanted to add operator=; because it should get the same
treatment as the copy constructor here :)
Besides, why are you doing all this nonsense in the assignment
operators? Simply assign the damn members and move on to more
important things...

Now I am beginning to suspect that there may be something special about
operator=. You agree that the compiler generated operator= is just right,
right? :)

Ali
 
J

Jacek Dziedzic

Victor said:
> [...]

Besides, why are you doing all this nonsense in the assignment
operators? Simply assign the damn members and move on to more
important things...

To quote Sutter and Alexandrescu:
"It is often beneficient to use the swap function in the
assignment operator. The below mentioned implementation gives
strong reliability, although at the price of constructing an
extra object. If speed matters it should not be used"

T& T::eek:perator=(const T& other) {
T temp(other);
swap(temp);
return *this;
}

So, if I understand correctly it means that if you make
the swap operation and the copy ctor unfailible (they don't
throw) that makes your assignment operator unfailible too.

That said, I think calling it "nonsense" is rather
strong worded.

Someone correct me if I'm wrong -- I only read the book
last week :)

cheers,
- J.
 
?

=?ISO-8859-2?Q?Ali_=C7ehreli?=

Jacek Dziedzic said:
Victor said:
[...]

Besides, why are you doing all this nonsense in the assignment
operators? Simply assign the damn members and move on to more
important things...

To quote Sutter and Alexandrescu:
"It is often beneficient to use the swap function in the
assignment operator. The below mentioned implementation gives
strong reliability, although at the price of constructing an
extra object. If speed matters it should not be used"

T& T::eek:perator=(const T& other) {
T temp(other);
swap(temp);
return *this;
}

Though uncommon but even better implementation is:

T& T::eek:perator=(T temp) // <-- note the copy
{
swap(temp);
return *this;
}

That version gives the compiler more help in optimizations.
So, if I understand correctly it means that if you make
the swap operation and the copy ctor unfailible (they don't
throw) that makes your assignment operator unfailible too.

No, only the swap needs to be non-throwing, and better be fast too (usually
is). Copy constructor can throw... The good thing is that, even if the copy
constructor throws, the state of the current object (the destination) is
unchanged. That is the power of doing the actual work on the side before
changing the state by non-throwing operations (swap in this case).

If additionally the copy constructor does not throw, then operator= does not
throw either.

Ali
 
J

Jacek Dziedzic

Ali said:
> [...]
Though uncommon but even better implementation is:

T& T::eek:perator=(T temp) // <-- note the copy
{
swap(temp);
return *this;
}

That version gives the compiler more help in optimizations.

Yep, that's what they mention on the next page :).
No, only the swap needs to be non-throwing, and better be fast too
(usually is). Copy constructor can throw... The good thing is that, even
if the copy constructor throws, the state of the current object (the
destination) is unchanged. That is the power of doing the actual work on
the side before changing the state by non-throwing operations (swap in
this case).

If additionally the copy constructor does not throw, then operator= does
not throw either.

Ah, thanks for the clarification!

cheers,
- J.
 
M

ma740988

Again... WHY? Just assign:

That's the preferred approach if I recall correctly from - I believe -
Sutter 's text.
No, not really. Since none of those objects require any special
processing during copy-construction, the compiler-generated one
is just right.
Actually, what I should have shown was an example were the msg class
dynamically allocated memory. In that case only the _msg_ class would
require an assignment operator?
 
V

Victor Bazarov

ma740988 said:
That's the preferred approach if I recall correctly from - I believe -
Sutter 's text.

Any justification for it that you actually understand? Following
somebody's "text" blindly has lead humanity to many a perils.
Actually, what I should have shown was an example were the msg class
dynamically allocated memory. In that case only the _msg_ class would
require an assignment operator?

See "the Rule of Three".

V
 
M

ma740988

Any justification for it that you actually understand? Following
somebody's "text" blindly has lead humanity to many a perils. :)

See "the Rule of Three".
Right and that I know. I was trying to get a feel for the impact with
regards to class A, B and C. B has a vector of A and C has a vector of
B. I think I understand now, in that - for the scenario, I
highlighted in my initial post. if class msg dynamically allocated
memory, then class msg should provided the operator=, etc and that
would be all I need even though I'm assigning 'controller' objects.

Thanks
 
G

Guest

Victor Bazarov said:
ma740988 wrote:

See "the Rule of Three".

The rule of three can be understood, as I've done in the past, as "don't
define operator= if the class consists only of members that take care of
themselves."

With "taking care of themselves," I mean classes that define proper copy
constructors and assignment operators, which work correctly. Further, let's
take "working correctly" here as copying and assigning successfully.
Propagating exceptions should be fine too, if the object is left in a
consistent state. For example, std::string is such a class.

If one applies the guidelines as above, and believes that their class will
work correctly even if a member's operator= can throw, they might be dealing
with corrupt objects.

I think that the rule of three must go with an addition:

- Observe the rule of three (If any one of the three special functions needs
to be defined, chances are you will have to at least declare the other two)

- Additionally, if you want your objects be in a consistent state if a
member throws, always define operator=.

I've run a simple program to analyze what happens during the execution of
operator= when different operations throw or not and when operator= is
implicit or explicit.

A: a class without any members
B: a class that contains two std::strings and an A that is deliberately
defined between two strings

class B
{
string first_;
A a_;
string last_;

/* ... */
};

The legend for the table below:

completes: The operation completes successfully
throws: The operation is interrupted with an exception
implicit: The compiler generated operator= is used
explicit: operator= is defined by the user
new: The object gets the new state
old: The object is left in the old state
corrupt: The state of the object is corrupt

The results are:

A::A(A) A::eek:perator=(A) B::eek:perator=(B) | B's state
------------------------------------------------------|------------
completes completes implicit | new
throws completes implicit | new
completes throws implicit | corrupt
throws throws implicit | corrupt
completes completes explicit | new
throws completes explicit | old
completes throws explicit | new
throws throws explicit | old

What I digest from these results is:

1) If you leave operator= to the compiler (implicit) you may end up with a
corrupt object if operator= of a member throws

2) If you define operator= yourself (explicit) you may be left with the old
state

3) Otherwise you will get the new state

As a result, operator= is even more special that the other two, in that, it
prevents having an inconsistent object.

Here is the program I used to test these:

#include <iostream>
#include <string>

using namespace std;

// Comment-out or keep combinations of these macros to see the
// behavior for each case
// #define A_COPY_THROWS
// #define A_ASSIGNMENT_THROWS
// #define B_ASSIGNMENT_EXPLICIT

class A
{
public:

A()
{}

#if defined(A_COPY_THROWS)
A(A const &)
{
throw 42;
}
#endif

#if defined(A_ASSIGNMENT_THROWS)
A & operator= (A const &)
{
throw 42;
}
#endif

void swap(A & other)
{}
};

class B
{
string first_;
A a_;
string last_;

friend ostream & operator<< (ostream &, B const &);

public:

explicit B(string const & value)
:
first_(value),
last_(value)
{}

bool operator== (B const & other) const
{
return ((first_ == other.first_)
&&
(last_ == other.last_));
}

#if defined(B_ASSIGNMENT_EXPLICIT)
B & operator= (B temp)
{
swap(temp);
return *this;
}
#endif

void swap(B & other)
{
first_.swap(other.first_);
a_.swap(other.a_);
last_.swap(other.last_);
}
};

ostream & operator<< (ostream & os, B const & b)
{
return os << b.first_ << ' ' << b.last_;
}

bool check_B_invariant(string const & label, B const & lhs, B const & rhs)
{
cout << label << ": ";

cout << "comparing " << lhs << " with " << rhs << '\n';

if ( ! (lhs == rhs))
{
cout << "NO ";
}

cout << "match\n";
}

int main()
{
B zero("0");
B b0("0");
B b1("1");

check_B_invariant("before", zero, b0);

try
{
b0 = b1;
check_B_invariant("completed", b0, b1);
}
catch (...)
{
check_B_invariant("caught exception", b0, zero);
}
}


Ali
 
G

Guest

Ali Çehreli said:
I think that the rule of three must go with an addition:

- Observe the rule of three (If any one of the three special functions
needs to be defined, chances are you will have to at least declare the
other two)

- Additionally, if you want your objects be in a consistent state if a
member throws, always define operator=.

Correction:

- Additionally, if you want your objects be in a consistent state if a
member throws _while_executing_operator=_, always define operator=.

Ali
 
M

ma740988

With "taking care of themselves," I mean classes that define proper copy
constructors and assignment operators, which work correctly. Further, let's
take "working correctly" here as copying and assigning successfully.
Propagating exceptions should be fine too, if the object is left in a
consistent state. For example, std::string is such a class.
Just so I understand. If std::string throws an exception, the string
object is left in a consistent state?

If one applies the guidelines as above, and believes that their class will
work correctly even if a member's operator= can throw, they might be dealing
with corrupt objects.

I think that the rule of three must go with an addition:

- Observe the rule of three (If any one of the three special functions needs
to be defined, chances are you will have to at least declare the other two)

- Additionally, if you want your objects be in a consistent state if a
member throws, always define operator=.

I've run a simple program to analyze what happens during the execution of
operator= when different operations throw or not and when operator= is
implicit or explicit.

A: a class without any members
B: a class that contains two std::strings and an A that is deliberately
defined between two strings

class B
{
string first_;
A a_;
string last_;

/* ... */
};

The legend for the table below:

completes: The operation completes successfully
throws: The operation is interrupted with an exception
implicit: The compiler generated operator= is used
explicit: operator= is defined by the user
new: The object gets the new state
old: The object is left in the old state
corrupt: The state of the object is corrupt

The results are:

A::A(A) A::eek:perator=(A) B::eek:perator=(B) | B's state
------------------------------------------------------|------------
completes completes implicit | new
throws completes implicit | new
completes throws implicit | corrupt
throws throws implicit | corrupt
completes completes explicit | new
throws completes explicit | old
completes throws explicit | new
throws throws explicit | old

What I digest from these results is:

1) If you leave operator= to the compiler (implicit) you may end up with a
corrupt object if operator= of a member throws

2) If you define operator= yourself (explicit) you may be left with the old
state

3) Otherwise you will get the new state

As a result, operator= is even more special that the other two, in that, it
prevents having an inconsistent object.

Here is the program I used to test these:

#include <iostream>
#include <string>

using namespace std;

// Comment-out or keep combinations of these macros to see the
// behavior for each case
// #define A_COPY_THROWS
// #define A_ASSIGNMENT_THROWS
// #define B_ASSIGNMENT_EXPLICIT

class A
{
public:

A()
{}

#if defined(A_COPY_THROWS)
A(A const &)
{
throw 42;
}
#endif

#if defined(A_ASSIGNMENT_THROWS)
A & operator= (A const &)
{
throw 42;
}
#endif

void swap(A & other)
{}
};

class B
{
string first_;
A a_;
string last_;

friend ostream & operator<< (ostream &, B const &);

public:

explicit B(string const & value)
:
first_(value),
last_(value)
{}

bool operator== (B const & other) const
{
return ((first_ == other.first_)
&&
(last_ == other.last_));
}

#if defined(B_ASSIGNMENT_EXPLICIT)
B & operator= (B temp)
{
swap(temp);
return *this;
}
#endif

void swap(B & other)
{
first_.swap(other.first_);
a_.swap(other.a_);
last_.swap(other.last_);
}
};

ostream & operator<< (ostream & os, B const & b)
{
return os << b.first_ << ' ' << b.last_;
}

bool check_B_invariant(string const & label, B const & lhs, B const & rhs)
{
cout << label << ": ";

cout << "comparing " << lhs << " with " << rhs << '\n';

if ( ! (lhs == rhs))
{
cout << "NO ";
}

cout << "match\n";
}

int main()
{
B zero("0");
B b0("0");
B b1("1");

check_B_invariant("before", zero, b0);

try
{
b0 = b1;
check_B_invariant("completed", b0, b1);
}
catch (...)
{
check_B_invariant("caught exception", b0, zero);
}
}
Thanks for the example.. I'll check it out in a minute.
 
K

Kai-Uwe Bux

ma740988 said:
Just so I understand. If std::string throws an exception, the string
object is left in a consistent state?

This is at least what Stroustrup claims in TC++PL-SE, Appendix E. I have not
been able to find a supporting clause in the standard, but that is probably
just my inability.

However, "consistent state" is a rather weak guarantee, just enough to make
sure the string can be properly destroyed.

Consider

class A {

std::string data;

public:

// whatever functions

};

vs.

class B {

std::string data;

public:

B ( void )
: data ()
{}

B ( B const & other )
: data ( other.data )
{}

B & operator= ( B const & other ) {
B tmp ( other );
std::swap( this->data, tmp.data );
return( *this );
}

// whatever functions are in A

};


The difference is that if

B b1;
B b2;
...
b1 = b2;

throws, you know that b1 did not change. In

A a1;
A a2;
...
a1 = a2;

a throw on the last line may modify a1 in unpredictable ways. In particular,
I can easily imagine an implementation of std::string::eek:perator= that uses
deep copy and first deletes the current buffer and then tries to allocate a
new buffer.


Best

Kai-Uwe Bux
 
L

Luke Meyers

Victor said:
Really? No, REALLY? Why not simply

someStr = rhs.someStr;
return *this;

?

Yes, really. Implementing operator= in terms of a a temporary copy
and a no-throw swap() function is a powerful idiom for ensuring the
basic exception-safety guarantee (pronounced "this code doesn't have
(that kind of) bugs"). It's true that in some cases, the members' copy
ctors are nothrow, or there is only one member so there's no state to
corrupt if it fails to copy (as string can). But when someone tries to
maintain this code, they're unlikely to change the whole structure of
operator= just because they've added a new member variable -- they're
just going to add it to the list and assume the original author set
things up acceptably. So, better to adopt this practice universally.
If you're worried about efficiency, first use a profiler to show that
there is an efficiency problem.
Besides, why are you doing all this nonsense in the assignment
operators? Simply assign the damn members and move on to more
important things...

See the 10-section miniseries on exception safety in "Exceptional C++"
by Herb Sutter to understand why your criticism is misplaced.
Error-safety is definitely on my list of important things. I was going
to refer you to the appropriate GotW (#8) in case you don't have access
to that text, but it's substantially less informative than the book.
I'd say buy or borrow the book. It's short and very helpful.

Luke
 
M

ma740988

Yes, really. Implementing operator= in terms of a a temporary copy
and a no-throw swap() function is a powerful idiom for ensuring the
basic exception-safety guarantee (pronounced "this code doesn't have
(that kind of) bugs").

With regards to a no-throw swap. What do you do when your class
contains constant members. So now:

class me {
int const var1;
int var2;
public:
me()
: var1(0x99)
, var2(0)
{}
me ( me const& other )
: var1 ( other.var1 )
, var2 ( other.var2 )
{}
me& operator= ( me const& other )
{
me tmp(other);
// std::swap(this->var1, tmp.var1 ); // OOPS
std::swap(this->var2, tmp.var2 );
}
};
 
G

Gavin Deane

ma740988 said:
With regards to a no-throw swap. What do you do when your class
contains constant members. So now:

class me {
int const var1;
int var2;
public:
me()
: var1(0x99)
, var2(0)
{}
me ( me const& other )
: var1 ( other.var1 )
, var2 ( other.var2 )
{}
me& operator= ( me const& other )
{
me tmp(other);
// std::swap(this->var1, tmp.var1 ); // OOPS

this->var1 = other.var1; // doesn't work either
std::swap(this->var2, tmp.var2 );
}
};

That's not an issue with the "create a temporary copy and swap" idiom.
That's a general issue with having constant members and an assignment
operater in the same class.

Since there is no way to change this->var1 in your assignment operator
anyway, you have to ask yourself what it means to be able to assign
objects of that type.

Gavin Deane
 
K

Kai-Uwe Bux

ma740988 said:
With regards to a no-throw swap. What do you do when your class
contains constant members. So now:

class me {
int const var1;
int var2;
public:
me()
: var1(0x99)
, var2(0)
{}
me ( me const& other )
: var1 ( other.var1 )
, var2 ( other.var2 )
{}
me& operator= ( me const& other )
{
me tmp(other);
// std::swap(this->var1, tmp.var1 ); // OOPS
std::swap(this->var2, tmp.var2 );
}
};

Hm, do you really think the swap() is at fault? How would you implement
operator=() for a class with const members anyway?


Best

Kai-Uwe Bux
 
M

ma740988

Hm, do you really think the swap() is at fault? How would you implement
operator=() for a class with const members anyway?

No, I'm not saying it's at fault but I realized I had a brain fart. As
Gavin pointed out, this:
this->var1 = other.var1; // doesn't work either
wouldn't work either.

I was just trying to create a _test_ case/class with a few of types
that required special handling. int *prt_member ( we can handle that
in operator=), static int, const int, volatile int ( and that's where I
stopped :)) and a contanier (std::vector<int> - no special handling
per-se but .. ) . Just something to debug and observe the behavior.
Something to build on what Ali wrote.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,482
Members
44,901
Latest member
Noble71S45

Latest Threads

Top