Assignment operator self-assignment check

Discussion in 'C++' started by Chris, Sep 20, 2006.

  1. Chris

    Chris Guest

    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.
     
    Chris, Sep 20, 2006
    #1
    1. Advertising

  2. Chris

    Phlip Guest

    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!!!
     
    Phlip, Sep 20, 2006
    #2
    1. Advertising

  3. 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
     
    Thomas Tutone, Sep 20, 2006
    #3
  4. 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::eek:perator=(MyClass const &rhs)
    {
    assert(this != &rhs);

    /* Now perform assignment */

    return *this;
    }

    --

    Frederick Gotham
     
    Frederick Gotham, Sep 20, 2006
    #4
  5. Chris

    S S Guest

    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::eek:perator=(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::shared_ptr<type_class> pt;
    };

    foo& foo::eek:perator=(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::eek:perator=(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
     
    S S, Sep 20, 2006
    #5
  6. Chris

    Noah Roberts Guest

    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.
     
    Noah Roberts, Sep 20, 2006
    #6
  7. 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
     
    Frederick Gotham, Sep 20, 2006
    #7
  8. Chris

    Noah Roberts Guest

    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.
     
    Noah Roberts, Sep 20, 2006
    #8
  9. Chris

    Gavin Deane Guest

    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
     
    Gavin Deane, Sep 20, 2006
    #9
  10. 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)return*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
     
    Frederick Gotham, Sep 20, 2006
    #10
  11. Chris

    Noah Roberts Guest

    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
     
    Noah Roberts, Sep 20, 2006
    #11
  12. Chris

    Old Wolf Guest

    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::eek:perator=( 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.
     
    Old Wolf, Sep 21, 2006
    #12
  13. Chris

    Kai-Uwe Bux Guest

    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
     
    Kai-Uwe Bux, Sep 21, 2006
    #13
  14. Chris

    Marcus Kwok Guest

    Gavin Deane <> wrote:
    > 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
     
    Marcus Kwok, Sep 21, 2006
    #14
  15. Chris

    LR Guest

    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
     
    LR, Sep 21, 2006
    #15
  16. Chris

    Guest

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


    This should very rarely be the case. It would violate the Assignable
    property for STL container elements. I would expect that some sort
    implementations can do self-assignments, so the STL has good
    reasons.

    When sorting ints, adding an extra int* comparision just in case
    would be rather expensive. Just assume x = x is cheap. Any class
    designer can add the obvious check if it really speeds things up.

    HTH,
    Michiel Salters
     
    , Sep 21, 2006
    #16
  17. Chris

    Kai-Uwe Bux Guest

    LR wrote:

    > 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?


    The copy-swap assignment idiom requires an overloaded swap method since the
    default std::swap function may use the assignment operator, which we just
    want to define.

    The code above is written under the assumption that swap( dummy&, dummy& )
    has been defined in the same namespace as dummy, in which case it should be
    found. Had I done

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

    it would be necessary to overload std::swap for dummy (which is allowed, and
    may be better practice -- I have not yet formed a firm opinion on that).


    Best

    Kai-Uwe Bux
     
    Kai-Uwe Bux, Sep 21, 2006
    #17
  18. Chris

    Kai-Uwe Bux Guest

    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.


    I think a reference counted non-intrusive smart pointer could benefit from
    the first version. Consider the following schematic implementation:

    template < typename T >
    class refcount_ptr;

    template < typename T >
    void swap ( refcount_ptr< T > &, refcount_ptr< T > & );

    template < typename T >
    class refcount_ptr {

    friend void swap<> ( refcount_ptr<T> &, refcount_ptr<T> & );

    struct T_count {

    T* t_ptr;
    unsigned long ref_count;

    T_count ( T * ptr )
    : t_ptr( ptr )
    , ref_count ( 1 )
    {}

    ~T_count ( void ) {
    delete ( t_ptr );
    }

    };

    T_count * c_ptr;

    public:

    refcount_ptr ( T * ptr = 0 )
    : c_ptr( new T_count ( ptr ) )
    {}

    refcount_ptr ( refcount_ptr const & other )
    : c_ptr ( other.c_ptr )
    {
    ++ c_ptr->ref_count;
    }

    ~refcount_ptr ( void ) {
    -- c_ptr->ref_count;
    if ( c_ptr->ref_count == 0 ) {
    delete( c_ptr );
    }
    }

    refcount_ptr & operator= ( refcount_ptr const & other ) {
    if ( c_ptr != other.c_ptr ) {
    this->~refcount_ptr();
    new ( this ) refcount_ptr( other );
    }
    return( *this );
    }

    T const * operator-> ( void ) const {
    return( c_ptr->t_ptr );
    }

    T * operator-> ( void ) {
    return( c_ptr->t_ptr );
    }

    // more stuff

    };

    In this case, the additional cost of the comparison could be zero: the
    refcount_ptr objects fit within registers. It might very well be that
    comparing the values of the c_ptr members is more efficient than comparing
    their addresses. On the other hand, the value comparison will safe on
    hidden self-assignments, which actually could be quite frequent depending
    on the application.


    Best

    Kai-Ue Bux
     
    Kai-Uwe Bux, Sep 21, 2006
    #18
  19. Noah Roberts posted:

    >> 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.



    Regardless of whether a "Hand" can recover from being assigned to itself,
    we can still prevent self-assignment in Debug Mode by using an "assert".


    > If you can recover then the check needs to happen anyway.



    I don't know where you're getting that from.


    > 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.



    I still don't see why you think it's invalid.


    >> 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.



    I don't need to do testing -- I know that there's a thing called "time",
    the basic unit of which is the second. I also know that there's a thing
    called "memory", the basic unit of which is the bit.


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



    The "Third Party" is the person who wrote the reusable code.


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

    >
    > Does it? By how much?



    This figure varies from system to system, compiler to compiler, and is
    influenced by numerous other things...


    >> If it's extremely bizarre that an object of a certain type be
    >> assigned 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.



    Not if we decide that an object of a particular type shouldn't be assigned
    to itself.


    > Besides, we are talking assignment, not comparison.



    I meant to write "assigned" rather than "compared". (I have correct the
    quote above.)


    > 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?



    Let's say that we're writing fast code. Let's say that, for some reason, it
    takes a long time to compare one hand to another, so we want to speed up
    the process as much as we can. Someone comes up with the idea of writing:

    if(this==&rhs)return*this;

    This will speed up considerably any self-comparison, but will slow down any
    proper comparison between different objects. Let's go on to say that, if a
    Hand is compared to itself (or perhaps assigned to itself) then that
    comparison algorithm will invoke undefined behaviour. An "assert" would
    work nicely to catch programming errors.


    > And there is no possibily for UB here.



    The behaviour is undefined if you compare two pointers which don't point
    into the same array. The following invokes UB:

    int array1[5], array2[5], *p1 = array1+3, *p2 = array2+2;

    p2 > p1; /* UB! */

    --

    Frederick Gotham
     
    Frederick Gotham, Sep 21, 2006
    #19
  20. Chris

    Noah Roberts Guest

    Frederick Gotham wrote:
    > Noah Roberts posted:
    >
    > >> 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.

    >
    >
    > I don't know where you're getting that from.


    Think about it. Your code was something like this:

    Obj & Obj::eek:perator = (Obj const & other)
    {
    assert (this != &other);

    do copy stuff...

    return *this;
    }

    Now, an assert will only catch the cases you test during debugging. It
    will not catch anything that happens under release. Therefor, when you
    are in release the check doesn't happen and the assigment op goes
    straight into "do copy stuff". If this is a self assignment "do copy
    stuff" probably explodes or reacts in some peculiar way. In order to
    keep this from affecting the user (through a crash or worse, bad
    results) the check has to be there anyway:

    Obj & Obj::eek:perator = (Obj const & other)
    {
    assert (this != &other);
    if (this == &other) return *this;

    do copy stuff...

    return *this;
    }

    So your argument that using an assert is more efficient doesn't hold
    water when you take care in the release build and acknowledge the FACT
    that some bugs will get through no matter what precautions you take.
    >
    >
    > > 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.

    >
    >
    > I still don't see why you think it's invalid.


    It's pointless. It also causes numerous problems by breaking the
    standard operation of operator =.
    >
    >
    > >> 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.

    >
    >
    > I don't need to do testing -- I know that there's a thing called "time",
    > the basic unit of which is the second. I also know that there's a thing
    > called "memory", the basic unit of which is the bit.


    Statements of performance should always be backed up by real numbers.
    What you are advocating is known as premature optimization. Not only
    that but it is a *micro* optimization and under almost all situations
    is utterly without merrit and won't have any affect on the speed of
    execution or memory footprint.
    >
    >
    > > First: What third party? Are you bringing someone new into the
    > > argument?

    >
    >
    > The "Third Party" is the person who wrote the reusable code.


    Your method of clipping makes it difficult to follow the conversation.
    Is that on purpose?

    The person who wrote the reusable code is the person under discussion
    so it isn't a *third* person, they are the *first* person. Code like
    that shouldn't be written.
    >
    >
    > >> Yes, but speed of execution suffers if self-assignment should trully be
    > >> a no-no.

    > >
    > > Does it? By how much?

    >
    >
    > This figure varies from system to system, compiler to compiler, and is
    > influenced by numerous other things...


    As I've argued, there is no difference given considerations of real
    world problems and the optimization you are trying to make will likely
    never be worth the problems. When it is, operator = should be disabled
    by making it private and the copy code should be in a function so as to
    advertize that this isn't a normal copy operator.

    > > 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?

    >
    >
    > Let's say that we're writing fast code. Let's say that, for some reason, it
    > takes a long time to compare one hand to another, so we want to speed up
    > the process as much as we can. Someone comes up with the idea of writing:
    >
    > if(this==&rhs)return*this;
    >
    > This will speed up considerably any self-comparison, but will slow down any
    > proper comparison between different objects. Let's go on to say that, if a
    > Hand is compared to itself (or perhaps assigned to itself) then that
    > comparison algorithm will invoke undefined behaviour. An "assert" would
    > work nicely to catch programming errors.


    Are we talking about comparison or assignment??? You keep using one
    where it appears you may need the other.

    if (this == &other) comparison == equal!
    if (this == &other) copy == noop.

    Now, you are asserting that doing those tests will somehow impact the
    performance of a given, highly theoretical program. Ok, let's assume
    that you are right. For some program somewhere, a very unique program,
    you need to optimize comparison and assignment so much that you cannot
    allow the normal safety checks to make it into release code. First,
    you damn well better have some real numbers to reach that conclusion.
    Second, such a micro optimization (three or four machine code
    operations at most) is the LAST thing to look at doing. Finally, you
    would certainly want to advertize the problem more clearly by disabling
    these operators and creating new ones so that nobody is confused about
    the fact that these are not the operators they are expecting.
    >
    >
    > > And there is no possibily for UB here.

    >
    >
    > The behaviour is undefined if you compare two pointers which don't point
    > into the same array. The following invokes UB:
    >
    > int array1[5], array2[5], *p1 = array1+3, *p2 = array2+2;
    >
    > p2 > p1; /* UB! */


    I don't really even know what you are talking about here as your
    clipping has completely removed context but I can see right off that
    there is no undefined behavior in the above code. I can't imagine why
    you think there is.
     
    Noah Roberts, Sep 21, 2006
    #20
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Ralf W. Grosse-Kunstleve
    Replies:
    16
    Views:
    620
    Lonnie Princehouse
    Jul 11, 2005
  2. Ralf W. Grosse-Kunstleve
    Replies:
    18
    Views:
    621
    Bengt Richter
    Jul 11, 2005
  3. Ralf W. Grosse-Kunstleve
    Replies:
    2
    Views:
    428
    Dan Sommers
    Jul 12, 2005
  4. falcon
    Replies:
    0
    Views:
    402
    falcon
    Jul 31, 2005
  5. Bart Kastermans
    Replies:
    6
    Views:
    425
    Bart Kastermans
    Jul 13, 2008
Loading...

Share This Page