tags:

views:

73

answers:

4

There is tree of classes rooted at CBase (single-inheritance). CSub is derived from CBase, and other classes derived from CBase or CSub, and CBase having virtual member functions.

All classes must be assignable. Some classes have members (pointers) that need special treatment in assignment (pointers). How do I ensure assignability within this tree of classes ?

First, I thought I need to make "operator=" virtual. Then I realized it's wrong. Then is this at all possible:

         CBase *x = new CSub; 
         CBase *y = new CSub; 
         *x = *y; // is this okay ?

If not, how do I assign *y to *x without evil downcast ? I have many questions here.

If I need to cast every time I assign through CBase*, then this does not look typesafe, does it ? Do I need to insert type checks in "operator=" to check that lhs and rhs have same type ? etc etc. Examples are welcome.

Thanks Viki

A: 

Why do they need to be assignable? You won't be able to do it safely/correctly and still allow for new subclassses, or you'll end up writing the code in a way where all the types of the objects are fully known at compile time and you can dodge the issue entirely. I would suggest alternatively, a virtual CBase* copy() const method. It can make use of covariant returns in the subclasses. No downcasting required, and no potential for slicing.

Logan Capaldo
A: 

The code you present does indeed perform "evil downcasting" (slicing things off like mad) -- it will always, "accidentally", work if CBase is a concrete class with a normal and usable assignment operator (and once again I repeat Haahr's advice: don't subclass [if you have a vtable or any extra data, i.e., any "serious" subclassing;-)] from concrete classes, only from abstract ones -- among other advantages you'll be saved from such accidental and misleading "cleanly compiling code" that's likely to do wrong things when it runs).

If you want every subclass in the tree to be assignable from an instance of any other (and if CBase has an assignment operator then by Liskov's principle so it should be), you'll need to make any subclass assignable from CBase and ensure it treats any subclass it doesn't know about by that route (ignoring whatever extra data the unknown sibling class may have added, sigh) -- or try a more virtualizable route with e.g. a virtual void assign(const CBase*) method (which does not require CBase to be concrete, btw;-).

This still doesn't deal with the problem of possibly having up to N squared special cases; for that, you might try Visitor or (probably better) Uncle Bob's Acyclic Visitor (still better to use that with named methods than with operator=, though;-).

Alex Martelli
+1  A: 

It's a simple question i believe. You'll need to implement "virtual assignment operator" which will require upcast imho:

class CBase {

...
   virtual assign(const CBase & from) = 0;
...
};

class CSub {
...
   void operator=(const CSub & from) { assign(from); }

   virtual assign(const CBase & from) {
      if (dynamic_cast<const CSub &>(from)) {
         ... implement simple copy here
      } else {
         ... you'll have to decide what to do with other types
      }
   }
};
Drakosha
A: 

Here you are dealing with pointers so you need to be careful:

 CBase *x = new CSub; 
 CBase *y = new CSub; 
 *x = *y; // is this okay ?

The above code causes a memory leak since x didn't free up the CSub object it points to.

When you deal with pointers you need to take care of previous content that they may point to.

You can use auto_ptr or some other smart pointer for this purpose:

std::auto_ptr<CBase> x( new CSub );
std::auto_ptr<CBase> y( new CSub );
x = y;    // x now owns the obj that y pointed to, x's old obj has been deleted

with the boost template library:

boost::shared_ptr<CBase> x( new CSub );
boost::shared_ptr<CBase> y( new CSub );
x = y; // now x and y point to same obj, old obj x pointed to has been deleted.

Ok that is not what you are asking, but thought I would cover that just in case.

To the assignment part of your question

To be on the safe side you need to create operator=() for each class. This is the way to control the assignment since you may have members that are pointers etc.

class CBase // should be made abstract
{
public:
   CBase() { ; }
   CBase& operator=(CBase& in) { copy members }
};

class CSub : public CBase
{
public:
  CSub() {;}
  CSub& operator=(CSub& in ) { copy members from in, and base class members }
};

hth

Anders K.