Assignment operator self-assignment check

[Chris]

Is there ever a reason to declare this as

if(*this == rhs)

as opposed to what I normally see

if(this == &rhs)

?

Seems like the former version is going to be more expensive rather than
simply comparing addresses as in the latter (plus the former requires
the class to define operator== as well, no?). Wondering if that added
effort is ever justified.

[Phlip]

Re: Assignment operator self-assignment check

Chris wrote:

Is there ever a reason to declare this as
>
if(*this == rhs)
>
as opposed to what I normally see
>
if(this == &rhs)
>
?

The self-assignment check detects object identity, which C++ specifies as an
object's addresses.

The degenerate situation of copying a value into an object which already had
that value is less important. Just let it happen; don't always waste CPU
cycles checking for it.

--
Phlip
http://www.greencheese.us/ZeekLand <-- NOT a blog!!!

[Thomas Tutone]

Re: Assignment operator self-assignment check

Chris wrote:

Is there ever a reason to declare this as
>
if(*this == rhs)
>
as opposed to what I normally see
>
if(this == &rhs)
>
?
>
Seems like the former version is going to be more expensive rather than
simply comparing addresses as in the latter (plus the former requires
the class to define operator== as well, no?). Wondering if that added
effort is ever justified.

Well, as long as we're dealing with hypotheticals: If (1) checking the
class for equality is cheap, (2) checking for equality frequently
evaluates to true even though it's not the same object (in the sense of
occuping the same address), and (3) using the copy constructor is very
expensive, then one can imagine using the former version, since it will
avoid unnecessary copy constructions.

The latter version ought to be the default, though.

Best regards,

Tom

[Frederick Gotham]

Re: Assignment operator self-assignment check

Chris posted:

Is there ever a reason to declare this as
>
if(*this == rhs)

That is not a declaration -- choose your words carefully.

as opposed to what I normally see
>
if(this == &rhs)

The latter compares the ADDRESSES of two objects, while the former compares
the two objects themselves.

Seems like the former version is going to be more expensive rather than
simply comparing addresses as in the latter (plus the former requires
the class to define operator== as well, no?).

Yes, the former requires an accessible operator==.

Wondering if that added effort is ever justified.

Depends on a wonderous amount of factors:
(1) How the class is implemented
(2) How expensive it is to copy it
(3) How expensive it is to compare it for equality
(4) How often an object is compared to itself

If an object of the class should NEVER be assigned to itself, I would
suggest an assert:

#include <cassert>

MyClass &MyClass::opera tor=(MyClass const &rhs)
{
assert(this != &rhs);

/* Now perform assignment */

return *this;
}

--

Frederick Gotham

[S S]

Re: Assignment operator self-assignment check


Frederick Gotham wrote:

Chris posted:
>

Is there ever a reason to declare this as

if(*this == rhs)

>
>
That is not a declaration -- choose your words carefully.
>
>

as opposed to what I normally see

if(this == &rhs)

>
>
The latter compares the ADDRESSES of two objects, while the former compares
the two objects themselves.
>
>

Seems like the former version is going to be more expensive rather than
simply comparing addresses as in the latter (plus the former requires
the class to define operator== as well, no?).

>
>
Yes, the former requires an accessible operator==.
>
>

Wondering if that added effort is ever justified.

>
>
Depends on a wonderous amount of factors:
(1) How the class is implemented
(2) How expensive it is to copy it
(3) How expensive it is to compare it for equality
(4) How often an object is compared to itself
>
If an object of the class should NEVER be assigned to itself, I would
suggest an assert:
>
#include <cassert>
>
MyClass &MyClass::opera tor=(MyClass const &rhs)
{
assert(this != &rhs);
>
/* Now perform assignment */
>
return *this;
}
>

I think assert should not be good solution as we may not want to assert
just in case someone did it by mistake.

Here we can use shared pointers

class foo {
private:
std::tr1::share d_ptr<type_clas spt;
};

foo& foo::operator=( const foo& rhs)
{
pt.reset(new type_class(*rhs .pt)); //reset deletes the first and points
to 2nd,
//if new throws exception reset will not implement and original will
not be deleted.
return *this;
}

OR the other way

foo& foo::operator=( const foo& rhs)
{
type_class *pBefore = pt;
pt = new type_class(rhs. pt);
delete pt;
return *this;
}

These approaches are better as they are taking care of self assignment
saftey as well as exception saftey.

-SS

--
>
Frederick Gotham

[Noah Roberts]

Re: Assignment operator self-assignment check


S S wrote:

I think assert should not be good solution as we may not want to assert
just in case someone did it by mistake.

Well, an assert should definately not be used here but not for that
reason. Self assignment is rather common for objects when a program
gets remotely interesting. Simply not doing the assignment or devising
a way that it will just not hurt anything are the correct options.

Now, your reasoning isn't right because an assert is placed in code for
exactly that reason...in case someone accidentally violates a
pre-condition. They are there for the developer and go away in release
mode.

What you don't want is an assert that is the sole way of avoiding a
situation unless such a situation cannot be resolved, as is not the
case here. So the assert would be ok if you wanted to impose such a
restriction but the normal checks should still apply in release mode
because this could happen under conditions that where not tested and
then the program would do stupid things...and the situation is very
resolvable, you just don't do the copy.

[Frederick Gotham]

Re: Assignment operator self-assignment check

S S posted:

I think assert should not be good solution as we may not want to assert
just in case someone did it by mistake.

An assert is the testing of a condition at runtime. If the condition is
true, nothing happens. If the condition is false:

(1) The program is terminated.
(2) The programmer is informed as to where they made their coding
error.

So you see, "assert" is used to catch programming errors quickly and to cut
down time on debugging.

Here we can use shared pointers

Overkill in my opinion.


Noah Roberts posted:

Well, an assert should definately not be used here but not for that
reason. Self assignment is rather common for objects when a program
gets remotely interesting. Simply not doing the assignment or devising
a way that it will just not hurt anything are the correct options.

If the inventor of the class deems that an object of the class should never
be assigned to itself, then an "assert" is quite appropriate. The situation
is quite akin to the following:

/* Function: CountOcc

This function counts the amount of occurrences
of a specific element in an array.

NB: (1) Neither argument may be a null pointer.
(2) The second pointer must be greater than the first.
*/

template<class T>
unsigned CountOcc(T const *pstart,T const*const pend,T const &elem)
{
assert(pstart); assert(pend); assert(pend pstart);

/* Rest of Code */
}

These asserts are very good practise in my opinion. The help the developer
without sacrificing speed of execution.

--

Frederick Gotham

[Noah Roberts]

Re: Assignment operator self-assignment check


Frederick Gotham wrote:

Noah Roberts posted:
>

Well, an assert should definately not be used here but not for that
reason. Self assignment is rather common for objects when a program
gets remotely interesting. Simply not doing the assignment or devising
a way that it will just not hurt anything are the correct options.

>
If the inventor of the class deems that an object of the class should never
be assigned to itself, then an "assert" is quite appropriate.

If such an inventor did indeed deem that such a pre-condition be the
case. However, that is an inappropriate precondition to enforce and
renders the class hardly usable. When designing the assignment
operator one should always be aware of, and account for (not disallow),
self-assignment. You should also try to at least provide the strong
guarantee. Both of these can often be killed with one stone simply by
accepting the parameter by value instead of by reference.

The situation

is quite akin to the following:
>
/* Function: CountOcc
>
This function counts the amount of occurrences
of a specific element in an array.
>
NB: (1) Neither argument may be a null pointer.
(2) The second pointer must be greater than the first.
*/
>
template<class T>
unsigned CountOcc(T const *pstart,T const*const pend,T const &elem)
{
assert(pstart); assert(pend); assert(pend pstart);
>
/* Rest of Code */
}
>
These asserts are very good practise in my opinion.

The first two probably, and notice how this differs a great deal from
the self assignment problem. Namely that at least the first two
pre-conditions are totally unresolvable; there is no correct answer if
those preconditions are not met whereas the answer to self assignment
is the object itself. The last isn't even valid in that the result of
the count of any specific element in an empty array should be 0, not a
blown assert...it also guarantees nothing about the validity of that
relationship between the two addresses.

Also interesting to note is that your sticking to C constructs has
resulted in a function that is not as generic as it could be. The use
of the iterator concept instead of pointers would result in a more
useful function (it could work with any container) and remove the
necissity, and in fact the possibility, of those asserts.

The fact that asserting that a ptr != 0 in many cases is rather useless
also in that the assert can pass without a valid pointer and a 0
pointer is the easiest invalid pointer to debug. Placing the assert to
advertize the precondition is enough for it to be there though.

[Gavin Deane]

Re: Assignment operator self-assignment check


Chris wrote:

Is there ever a reason to declare this as
>
if(*this == rhs)
>
as opposed to what I normally see
>
if(this == &rhs)
>
?
>
Seems like the former version is going to be more expensive rather than
simply comparing addresses as in the latter (plus the former requires
the class to define operator== as well, no?). Wondering if that added
effort is ever justified.

Why are you testing for self-assignment at all? What does the rest of
your assignment operator look like? Can you implement your operator= as
"create a temporary and swap" as in http://www.gotw.ca/gotw/059.htm.

Gavin Deane

[Frederick Gotham]

Re: Assignment operator self-assignment check

Noah Roberts posted:

If such an inventor did indeed deem that such a pre-condition be the
case. However, that is an inappropriate precondition to enforce and
renders the class hardly usable.

I would say that that depends entirely on the kind of class we're
dealing with. Let's say we have a card game, and that we have a class
called "Hand". Let's say that we code the game in such a way that a hand
should never be compared to itself -- in fact, let's say that it would be
quite perverse if it were. In such circumstances, the "assert" solves two
problems:

(1) It informs the programmer of their coding error in Debug Mode.
(2) It doesn't burdeon the valid code in Release Mode with a decrease
in speed of execution.

When designing the assignment operator one should always be aware of,
and account for (not disallow), self-assignment.

I believe that rule is too rigid. I don't keep many rules in mind when
programming, I like to be fluidic and open to all possibilities -- it
results in more creative code (which tends to run faster too).

You should also try to at least provide the strong guarantee. Both of
these can often be killed with one stone simply by accepting the
parameter by value instead of by reference.

If speed of execution and memory consumption are no object, then yes.


<snip code>

>These asserts are very good practise in my opinion.

>
The first two probably, and notice how this differs a great deal from
the self assignment problem.

I believe they are very similar: They deal with something that the 3rd
party has outlawed. In the previous example, the 3rd party outlawed self-
assignment. In _this_ example, the 3rd party outlaws the passing of invalid
pointers.

Namely that at least the first two pre-conditions are totally
unresolvable; there is no correct answer if those preconditions are not
met whereas the answer to self assignment is the object itself.

Yes, but speed of execution suffers if self-assignment should trully be a
no-no. If it's extremely bizarre that an object of a certain type be
compared to itself, then we can just outlaw the practise by using an
"assert". If however it's not too weird that an object of a certain type be
compared to itself, then just go with:

if(this==&rhs)r eturn*this;

The last isn't even valid in that the result of the count of any
specific element in an empty array should be 0, not a blown assert...

This depends on whether the 3rd party considers an empty array to be an
array.

it also guarantees nothing about the validity of that relationship
between the two addresses.

Acknowledged, however it does guarantee that "pend" is ahead of "pstart"
(assuming of course that their comparisson doesn't invoke UB ; ) ).

Also interesting to note is that your sticking to C constructs has
resulted in a function that is not as generic as it could be. The use
of the iterator concept instead of pointers would result in a more
useful function (it could work with any container) and remove the
necissity, and in fact the possibility, of those asserts.

I have yet to read up on iterators; can you suggest a good book? I tried
TC++PL but didn't like the way it approached the Standard Library.

The fact that asserting that a ptr != 0 in many cases is rather useless
also in that the assert can pass without a valid pointer and a 0
pointer is the easiest invalid pointer to debug. Placing the assert to
advertize the precondition is enough for it to be there though.

Sorry I don't quite understand what you're saying in that last sentence.

--

Frederick Gotham

[Noah Roberts]

Re: Assignment operator self-assignment check


Frederick Gotham wrote:

In such circumstances, the "assert" solves two
problems:
>
(1) It informs the programmer of their coding error in Debug Mode.
(2) It doesn't burdeon the valid code in Release Mode with a decrease
in speed of execution.

Well, if it truely is not recoverable then #2 is true, but not
otherwise. If you can recover then the check needs to happen anyway.
Such is the case in self assignment and as you'll see when you go back
and read my original reply that is the primary reason an assert here is
invalid.

>
>

When designing the assignment operator one should always be aware of,
and account for (not disallow), self-assignment.

>
>
I believe that rule is too rigid. I don't keep many rules in mind when
programming, I like to be fluidic and open to all possibilities -- it
results in more creative code (which tends to run faster too).

Yes, I know that you don't follow a lot of the procedures used by
professionally minded programmers, such as adhering to coding
standards. This is a bad thing actually but it may be a while before
you realize this.

You should also try to at least provide the strong guarantee. Both of
these can often be killed with one stone simply by accepting the
parameter by value instead of by reference.

>
>
If speed of execution and memory consumption are no object, then yes.

If you say so. I think you should go do some more reading and testing
before making that assumption.

>
>
<snip code>

These asserts are very good practise in my opinion.

The first two probably, and notice how this differs a great deal from
the self assignment problem.

>
>
I believe they are very similar: They deal with something that the 3rd
party has outlawed.

First: What third party? Are you bringing someone new into the
argument?

And if you don't see the difference then you got problems.

Namely that at least the first two pre-conditions are totally
unresolvable; there is no correct answer if those preconditions are not
met whereas the answer to self assignment is the object itself.

>
>
Yes, but speed of execution suffers if self-assignment should trully be a
no-no.

Does it? By how much?

If it's extremely bizarre that an object of a certain type be
compared to itself,

Anyone that has ever worked on a long term and/or large project in a
team understands that such assumptions are bad assumptions to make.

Besides, we are talking assignment, not comparison.

But let's look at your example with a card hand. A card hard should
always be equal to itself no? Is there any condition in which it
wouldn't be? So why is it so perverse that you compare it to itself?
What are the concequences if it is? With an assert and no accounting
for the possibility in release mode the concequence is that such a
check could make it into production code and do unexpected things (not
so much a possibilty with comparisons but a major problem with
assignment). If the situation is just accounted for then it is just
never a problem...less debugging and no chance for post-release bug
having to do with this kind of operation.

So, the profile had better show some significant and profound impact in
the check to want to remove it since the cost in productivity and risk
of release bugs is so much higher.

The last isn't even valid in that the result of the count of any
specific element in an empty array should be 0, not a blown assert...

>
>
This depends on whether the 3rd party considers an empty array to be an
array.

Who is this third party you refer to?

Such policy should be enforced at a higher level. Enforcing it at this
level requires code duplication if such a function could be used on an
empty array.

>
>

it also guarantees nothing about the validity of that relationship
between the two addresses.

>
>
Acknowledged, however it does guarantee that "pend" is ahead of "pstart"
(assuming of course that their comparisson doesn't invoke UB ; ) ).

So what?

And there is no possibily for UB here.

Also interesting to note is that your sticking to C constructs has
resulted in a function that is not as generic as it could be. The use
of the iterator concept instead of pointers would result in a more
useful function (it could work with any container) and remove the
necissity, and in fact the possibility, of those asserts.

>
>
I have yet to read up on iterators; can you suggest a good book? I tried
TC++PL but didn't like the way it approached the Standard Library.

The C++ Standard Library: a tutorial and reference
The C++ Standard

[Old Wolf]

Re: Assignment operator self-assignment check

Chris wrote:

Is there ever a reason to declare this as
>
if(*this == rhs)

That's a statement, not a declaration.

>
as opposed to what I normally see
>
if(this == &rhs)
>
?
>
Seems like the former version is going to be more expensive rather than
simply comparing addresses as in the latter (plus the former requires
the class to define operator== as well, no?). Wondering if that added
effort is ever justified.

You're missing the point of the check. This check is to prevent
assigning an object to itself.

For example, suppose the code is something like:
Type::operator= ( Type const &old )
{
m_vec.clear();
copy( old.m_vec.begin (), old.m_vec.end() , back_inserter(m _vec) );
}

That code will just delete the vector's contents if you try assigning
an object to itself. Hence the simple, inexpensive check of address.

[Kai-Uwe Bux]

Re: Assignment operator self-assignment check

Gavin Deane wrote:

>
Chris wrote:

>Is there ever a reason to declare this as
>>
>if(*this == rhs)
>>
>as opposed to what I normally see
>>
>if(this == &rhs)
>>
>?
>>
>Seems like the former version is going to be more expensive rather than
>simply comparing addresses as in the latter (plus the former requires
>the class to define operator== as well, no?). Wondering if that added
>effort is ever justified.

>
Why are you testing for self-assignment at all? What does the rest of
your assignment operator look like?

Before class specific optimization, it could generically look like this:

dummy & operator= ( dummy const & rhs ) {
if ( this != &rhs ) {
this->~dummy();
new ( this ) dummy ( rhs );
}
return ( *this );
}

Can you implement your operator= as "create a temporary and swap" as in
http://www.gotw.ca/gotw/059.htm.

Well, that would be

dummy & operator= ( dummy rhs ) {
swap( *this, rhs );
return ( *this );
}

which also calls a destructor and a copy constructor. Just the order is
different. On top of that, it calls swap, which usually is not very
expensive. The main advantage is that it provides the strong exception
safety guarantee.

Both generic solutions, however, can be prohibitively expensive. Consider
for instance the std::vector template. In this case, you would not want to
create a complete copy someplace elsewhere in memory when maybe the
original lhs had sufficient capacity. That kind of optimization will not be
achieved by any generic approach. There are places (and some would maintain
that there are quite a few such places) where the strong guarantee should
be abandoned in favor of a more efficient assignment. After all, if one
needs the strong guarantee in a certain place, one could always use

template < typename T >
T& safe_assign ( T& lhs, T rhs ) {
swap( lhs, rhs );
return ( lhs );
}

That way would be more in line with: you don't pay for what you don't use.


Best

Kai-Uwe Bux

[Marcus Kwok]

Re: Assignment operator self-assignment check

Gavin Deane <deane_gavin@ho tmail.comwrote:

Why are you testing for self-assignment at all? What does the rest of
your assignment operator look like? Can you implement your operator= as
"create a temporary and swap" as in http://www.gotw.ca/gotw/059.htm.

All this talk of the assignment operator reminds me of these articles
talking about the issues with making an exception-safe assignment
operator:

http://icu.sourceforge.net/docs/papers/cpp_report/the_anatomy_of_the_assignment_operator.html

http://icu.sourceforge.net/docs/papers/cpp_report/the_assignment_operator_revisited.html

--
Marcus Kwok
Replace 'invalid' with 'net' to reply

[LR]

Re: Assignment operator self-assignment check

Kai-Uwe Bux wrote:

>>Can you implement your operator= as "create a temporary and swap" as in
>>http://www.gotw.ca/gotw/059.htm.

>
>
Well, that would be
>
dummy & operator= ( dummy rhs ) {
swap( *this, rhs );
return ( *this );
}
>
which also calls a destructor and a copy constructor. Just the order is
different. On top of that, it calls swap, which usually is not very
expensive. The main advantage is that it provides the strong exception
safety guarantee.

I feel a little confused. Which version of swap are you calling?

LR