Templated assignment operator question

Question

I want to make sure that *this != &rhs in the assignment operator. But it won't compile. Any suggestions?

template <typename T>
class A {
  public:
      A() {
          std::cout << "Default Constructor" << std::endl;
      }

      A(const T& t) : m_t(t) {
          std::cout << "Templated Constructor" << std::endl;
      }

      template <typename X>
      A( const A<X>& rhs ) : m_t( (static_cast< A<T> >(rhs)).m_t ) {
            std::cout << "Copy Constructor" << std::endl;
      }

      template <typename X>
      const A& operator=( A<X>& rhs) {
            std::cout << "Assignment Operator" << std::endl;
            if (this != static_cast< A<T>* > (&rhs) )
                m_t = rhs.get();
            return *this;
      }

      T get() { return m_t; }
  private:
      T m_t;
};


class base {};
class derived : public base {};


int main()
{
    A<base*> test1;
    A<derived*> test2;
    test1 = test2;  
}

Answer 1

What you are trying to do here

if (this != static_cast< A<T>* > (&rhs) )

is perform a static_cast from a A<derived*> to a A<base*>.

Here's what static_cast does:

You can explicitly convert a pointer of a type A to a pointer of a type B if A is a base class of B. If A is not a base class of B, a compiler error will result.

A<base*> is not a base class of A<derived*>, hence the error.

In general, obviously A<T> will never be a base class of A<X>, even if X is convertible to T. So that cast is out of the equation.

A solution would be to use reinterpret_cast<void*>(&rhs) instead.

Update

I worked on this some more; here are the results. I 'll give the code first, then comment.

Setup code

template <typename T>
class Aggregator {
  public:
      Aggregator() {
          std::cout << "Default Constructor" << std::endl;
      }

      Aggregator(const T& t) : m_t(t) {
          std::cout << "Constructor With Argument" << std::endl;
      }

      Aggregator& operator= (const Aggregator& rhs)
      {
          std::cout << "Assignment Operator (same type)";
          if (this->get() == rhs.get()) {
              std::cout << " -- SKIPPED assignment";
          }
          else {
              T justForTestingCompilation = rhs.get();
          }
          std::cout << std::endl;
          return *this;
      }

      template <class U>
      Aggregator& operator=(const Aggregator<U>& rhs)
      {
          std::cout << "Assignment Operator (template)";
          if (this->get() == rhs.get()) {
              std::cout << " -- SKIPPED assignment";
          }
          else {
              T justForTestingCompilation = rhs.get();
          }
          std::cout << std::endl;
          return *this;
      }

      T get() const { return m_t; }
  private:
      T m_t;
};


class base {};
class derived : public base {};
class unrelated {};

// This is just for the code to compile; in practice will always return false
bool operator==(const base& lhs, const base& rhs) { return &lhs == &rhs; }

The important points on what is going on so far:

1. I don't have any copy constructor. In real life, we 'd move the assignment logic into the copy constructor and implement the assignment operator with copy and swap. Keeping the code short(er) for now.
2. The assignment operators don't actually do anything, but they will compile iff their "normal", do-what-you-should version would.
3. There are two assingment operators; the first one for assigning Aggregate<T> to Aggregate<T> and the second for assigning Aggregate<T1> to Aggregate<T2>. This is straight from Tony's answer, and it's "the right way".
4. The free operator== is there to fake a comparison operator for types where one is not implicitly defined. Specifically, we need that for code that contains Aggregate<U> to compile when U is not a primitive type.

Excercise code

Here's where all the fun is:

int main(int argc, char* argv[])
{
    base b;
    derived d;
    unrelated u;

    Aggregator<base*> aggPB(&b);
    Aggregator<base*> aggPBDerivedInstance(&d);
    Aggregator<derived*> aggPD(&d);
    Aggregator<unrelated*> aggPU(&u);

    Aggregator<base> aggB(b);
    Aggregator<base> aggBDerivedInstance(d); // slicing occurs here
    Aggregator<derived> aggD(d);
    Aggregator<unrelated> aggU(u);

    std::cout << "1:" << std::endl;

    // base* = base*; should compile, but SKIP assignment
    // Reason: aggregate values are the same pointer
    aggPB = aggPB;

    // base = base; should compile, perform assignment
    // Reason: aggregate values are different copies of same object
    aggB = aggB;

    std::cout << "2:" << std::endl;

    // base* = base*; should compile, perform assignment
    // Reason: aggregate values are pointers to different objects
    aggPB = aggPBDerivedInstance;

    // base = base; should compile, perform assignment
    // Reason: aggregate values are (copies of) different objects
    aggB = aggBDerivedInstance;

    std::cout << "3:" << std::endl;

    // base* = derived*; should compile, perform assignment
    // Reason: aggregate values are pointers to different objects
    aggPB = aggPD;

    // base = derived; should compile, perform assignment (SLICING!)
    // Reason: derived is implicitly convertible to base, aggregates are (copies of) different objects
    aggB = aggD;

    std::cout << "4:" << std::endl;

    // base* = derived*; should compile, but SKIP assignment
    // Reason: aggregate values are (differently typed) pointers to same object
    aggPBDerivedInstance = aggPD;

    // base = derived; should compile, perform assignment (SLICING!)
    // Reason: derived is implicitly convertible to base, aggregates are (copies of) different objects
    aggBDerivedInstance = aggD;

    std::cout << "5:" << std::endl;

    // derived* = base*; should NOT compile
    // Reason: base* not implicitly convertible to derived*
    // aggPD = aggPB;

    // derived = base; should NOT compile
    // Reason: base not implicitly convertible to derived
    // aggD = aggB;

    return 0;
}

This will output:

Constructor With Argument
Constructor With Argument
Constructor With Argument
Constructor With Argument
Constructor With Argument
Constructor With Argument
Constructor With Argument
Constructor With Argument
1:
Assignment Operator (same type) -- SKIPPED assignment
Assignment Operator (same type)
2:
Assignment Operator (same type)
Assignment Operator (same type)
3:
Assignment Operator (template)
Assignment Operator (template)
4:
Assignment Operator (template) -- SKIPPED assignment
Assignment Operator (template)
5:

So... what do we learn from this?

1. The templated assignment operator, as written, will not compile unless there's an implicit conversion between the types aggregated. That's a good thing. This code will fail to compile rather than crash on you.
2. The test for equality only saved us two assignments, and both of them are pointer assignments (which are so cheap we need not have checked).

I 'd say this means that the equality check is superfluous and should be removed.

However, if:

1. T1 and T2 are the same type or an implicit conversion exists, and
2. T1's assignment operator or T2's conversion operator is expensive (this immediately takes primitives out of the picture), and
3. A bool operator== (const T1& lhs, const T2& rhs) that has a much smaller runtime cost than the above assignment/conversion operators

then checking for equality might make sense (depends on how often you would expect operator== to return true).

Conclusion

If you intend to use Aggregator<T> just with pointer types (or any other primitive), then the equality tests are superfluous.

If you intend to use it with expensive to construct class types, then you 'll need a meaningful equality operator to go with them. If you use it with different types as well, conversion operators will also be required.

Answer 2

If it really bothers you, you can always have a second non-templated operator= that doesn't need the cast. To avoid redudancy, if this != &rhs it can explicitly invoke the template version. Example illustrating how the right operator gets called:

#include <iostream>

template <class T>
struct X
{
    X& operator=(X& rhs)
    {
        std::cout << "non-template " << (this == &rhs ? "self\n" : "other\n");
    }

    template <class U>
    X& operator=(X<U>& rhs)
    {
        std::cout << "template\n";
    }
};

int main()
{
    X<int> x;
    x = x;
    X<int> y;
    x = y;
    X<double> z;
    x = z;
}

Answer 3

There is no need to test for self-assignment in your case. Self-assignment tests, contrary to what some tutorials may suggest, are not essential to overloaded assignment operators in general.

That is only needed if the (poorly implemented) assignment operator first releases the resources and then creates new resources to be the copy of the right-hand operarand's resources. Only then would self-assignment turn out to be catastrophic, because the left-hand object would have released the resources of the right-hand operand (itself) at the same time.

poor_assignment& operator=(const poor_assignment& rhv)
{
    this->Release(); // == rhv.Release() in case of self-assignment
    this->Create(rhv.GetResources()); 
}

Answer 4

The good version: Implement the copy assignment operator that takes exactly the same type as the class itself:

const A& operator=( A<T>& rhs) {
    std::cout << "copy assignment operator" << std::endl;
    if(this != &rhs)
        m_t = rhs.m_t;
    return *this;
}

The 'dirty' version: Cast the address of each object to a intptr_t and compare the plain values:

template<class X>
const A& operator=( A<X>& rhs) {
    std::cout << "Assignment Operator" << std::endl;
    if((intptr_t)(this) != (intptr_t)(&rhs))
        m_t = rhs.get();
    return *this;
}

Edit: Actually, this second version won't work, because of the compiler generated copy assignment operator that is used instead. So just implement one yourself. --end edit
Also, you don't need to use rhs.get(), just use rhs.m_t, as you can access private members from the class itself.

Answer 5

I personally think the following is the most elegant solution. I am not sure why I didn't get that in the first place - I initially was using the bloodshed c++ compiler and it seemed to fail - but g++ this is cleanest?

If people disagree with me, I'll remove my answer and give it to someone else. Note that the A* actually means A* but is note required

  template <typename X>
  const A& operator=( A<X>& rhs) {
        std::cout << "Assignment Operator" << std::endl;
        if (this != reinterpret_cast< A* >(&rhs))  
            m_t = rhs.get();               
        return *this;
  }