abstract base class and clone

Started by
18 comments, last by snooty 17 years, 6 months ago
I've been wrestling with abstract base class and a clone method, but can't make it work without sacrificing type-safety. Here's the problem:

class Abstract {
public:
   virtual void clone(const Abstract &A) = 0;
};

class Concrete : public Abstract {
private:
   int i;

public:
   void clone(const Concrete &C) {...}
   //this function is considered by the compiler to be different from the virtual function in Abstract

   void clone(const Abstract &A) {...}
   //in this function, A needs to be cast to Concrete in order to access i, which is not type-safe
   //also, it accepts an Abstract object from the user, which shouldn't be allowed
};


Please help.
Advertisement
search your compiler documentation and enable C++ RTTI (Run Time Type Info) - be warnedm, this adds additional overhead to each class and needs to be carefully used.

simple example code:
#include <string>#include <exception>#include <iostream>class Abstract {public:   virtual void clone(const Abstract &A) = 0;};class cast_exception : public std::exception{  std::string from, to;public:  virtual ~cast_exception() throw() {}  cast_exception( std::string from_, std::string to_ ) : from(from_), to(to_) {}  virtual const char* what() const throw() {    return (std::string("Class Cast Exception, Tried to cast from ")           + from + std::string(" to ") + to).c_str();  }};class Concrete : public Abstract {private:   int i;public:   Concrete(int i_) : i(i_) {}   //void clone(const Concrete &C) {...}   //this function is considered by the compiler to be different from the virtual function in Abstract   void clone(const Abstract &A)   {      const Concrete* c_ptr = dynamic_cast< const Concrete* >( &A );      if( c_ptr == NULL ) {          throw cast_exception("Abstract", "Concrete");      }      this->i = c_ptr->i;   }   const int get() const { return i; }};class DifferentConcrete : public Abstract {public:   float j;   DifferentConcrete( float j_ ) : j(j_) {}   const float get() const { return j; }   virtual void clone(const Abstract &A) {}};int main(){	Concrete *c = new Concrete(25);	Concrete *b = new Concrete(50);	DifferentConcrete *d = new DifferentConcrete(23.32f);    try {		b->clone(*c);		b->clone(*d);	} catch( std::exception& e ) {		std::cout << e.what() << std::endl;	}	return 0;}
Compile with g++ -W -Wall filename.cpp
"I am a donut! Ask not how many tris/batch, but rather how many batches/frame!" -- Matthias Wloka & Richard Huddy, (GDC, DirectX 9 Performance)

http://www.silvermace.com/ -- My personal website
I had thought clone methods were implemented like this:

class Abstract{public:   virtual Abstract * clone() = 0;};class Concrete1 : public Abstract{public:   virtual Abstract * clone() { return new Concrete1(*this); }};class Concrete2 : public Abstract{public:   virtual Abstract * clone() { return new Concrete2(*this); }};Abstract * p1;Abstract * p2 = new Concrete1;Abstract * p3 = new Concrete2;p1 = p2->clone(); // p1 now points to a Concrete1 objectdelete p1;p1 = p3->clone(); // p1 now points to a Concrete2 objectdelete p2;


This way, type safety is preserved.
Quote:Original post by snooty
I've been wrestling with abstract base class and a clone method, but can't make it work without sacrificing type-safety.
Here's the problem:

*** Source Snippet Removed ***

Please help.


You seems to be confused about the prototype pattern (the name of this design pattern). This is because you try to clone the function parameter, while you should clone the current object. Ie instead of writing p3 = p1->clone(*p2), you should write p3 = p2->clone(). This way, as Rainault spotted, you only care about the type of p2 - and this is where the virtual function comes to help, because the type of p2 is known in when cloning.

HTH,
Thank you for the replys.

Actually, what I want is for the Concrete class to have a copy constructor. My way of doing it is to call the clone method in the copy constructor to do the copying.

The RTTI way looks ok, I'll try it. But if it is causing a serious performance hit, I think I'll resort to commenting to warn users about this instead. Probably a bad way, but may be the only way...
Quote:Original post by snooty
Thank you for the replys.

Actually, what I want is for the Concrete class to have a copy constructor. My way of doing it is to call the clone method in the copy constructor to do the copying.

The RTTI way looks ok, I'll try it. But if it is causing a serious performance hit, I think I'll resort to commenting to warn users about this instead. Probably a bad way, but may be the only way...


You won't be able to call a virtual function from a constructor. The vtable is not yet defined, meaning that you'll always end up by calling the function which is defined in the class itself (in most implementations of the standard). However, I really strongly suggest you to rethink about this approach, as it is potentially dangerous.

Moreover, cloneing is not copying, as far as the semantic goes. It is perfectly safe to have a private, non virtual copy() function that will do the work for you (and that can be reused in operator=()).

To finish, constructor should use an initialization list everytime it is possible. It is less computationally intensive, as the variable members are initialized at construction, instead of being initialized by a call to their operator=. Consider the case where member a of type A is a complex type, doing:
class B{  A a;public:  // a is created and initialized in one operation  B(const B& b) : a(b.getA()) { }  const A& getA() const { return a; }};

vs.
class B{  A a;public:  // call the default constructor of A, then operator=()  // that's slower than the previous code.  B(const B& b) { a = b.getA(); }  const A& getA() const { return a; }};

My advice is to not try to do fancy stuff here. No offense intended but you seems to show a lack of knowledge regarding how C++ works, so in your position I'd do the simplest thing that can possibly work - which in your case is a basic implementation of the copy constructor.

HTH,
I'm very sorry about this, but I don't understand why calling virtual functions in constructors is problematic. Maybe I wasn't expressing myself clearly. Maybe I did a bad job abstracting what I am really trying to do.

So, please allow me to clarify the whole thing, this time using the real program that I'm trying to write.

I want to write a library that contain classes that handle 2D drawing on bitmaps. I use templates to handle different color formats of the bitmap:
enum FORMAT { F555, F888 };template<enum FORMAT f>class Image {private:   UINT *bits;   //an array that stores the bitmap bits};

Everything is fine so far, but then I want to write a class that handles a bitmap compatible to the display format, which is determined at runtime. I let the former Image class derive from an abstract base class so that the new DisplayImage class can store a pointer to it:
class Base { };template<enum FORMAT f>class Image : public Base {private:   UINT *bits;};class DisplayImage : public Base {private:   Base *img;};

Now, both Image and DisplayImage have a copy constructor and a copy method:
class Base {public:   virtual void Copy(const Base &b) = 0;};template<enum FORMAT f>class Image : public Base {public:   void Copy(const Base &b) { ... }   //PROBLEM: accepts Image<F555> and Image<F888>,                                      //but an instance of Image<F555> shouldn't accept Image<F888>, and vice versa   Image(const Image &i) { Copy((const Base)i); }   //PROBLEM: calling virtual function in constructor, ???private:   UINT *bits;};FORMAT format = F888;   //set before DisplayImage is constructedclass DisplayImage : public Base {public:   void Copy(const Base &b) { img->Copy( ((const DisplayImage)b).img ); }   //PROBLEM: using non-typesafe cast   DisplayImage(const DisplayImage &di) {      switch (format) {         case F888: img = new Image<F888>(*(Image<F888> *)di.img); break;   //PROBLEM: using non-typesafe cast         case F555: img = new Image<F555>(*(Image<F555> *)di.img); break;   //PROBLEM: using non-typesafe cast      }   }private:   Base *img;};

Please don't give me up. :)

[Edited by - snooty on October 18, 2006 1:49:02 PM]
Is an inheritance hierarchy really necessary here? I don't get the point of some of your classes. What does Base represent? What's the difference between Image and DisplayImage? Why does DisplayImage have a pointer to a Base? Give your justifications for these classes; you may need to do a little class redesign. If the only purpose of Base is to provide a pure virtual Copy method, I don't think you really need inheritance here.
Quote:
I'm very sorry about this, but I don't understand why calling virtual functions in constructors is problematic.


Because it just doesn't work. During construction, the type of the object is the type of class for which the current constructor is executing. Virtual dispatch within a constructor of type T cannot dispatch to anything "below" type T in the inheritance hierarchy -- the object is not that type yet, regardless of whether or not it ultimately will be (and because of that, many compilers will emit code to directly call the appropriate methods rather than attempt to perform a v-table dispatch, because the result of the dispatch in known at compile time).

It's not hard see why this is true. If dispatch were "allowed" to happen in a constructor, it still wouldn't work the way you wanted it to because you would be calling a method of a derived type that read or manipulated data members that, since none of the derived type's constructors had run yet, were not even initialized. It would thus be impossible to guarantee class invariants in any virtual function, rendering them so dangerous as to be completely useless, especially for production code.

EDIT:
So, after looking over your code, I raise the same questions as Rainault.
It seems to me that all you need is:
template< FORMAT F_ >class Image{  public:    // copy ct    Image(const Image &other)    {      /* implementation here... */    }};


Given the additional context you've provided here, I don't actually see a need for any kind of inheritance whatsoever. Base is useless. Holding on to pointers to Base in the derived classes is pointless. DisplayImage is useless. I'm not neccessarily sure there's even any benefit to making the format a template argument... but I'll let that slide.

Can you explain, in a larger context, what these classes are for and how they are used in a program?

[Edited by - jpetrie on October 18, 2006 2:25:25 PM]
RTTI is totally not called for here.

Quote:Original post by snooty
Actually, what I want is for the Concrete class to have a copy constructor. My way of doing it is to call the clone method in the copy constructor to do the copying.


As noted, a virtual function isn't going to work properly from within a constructor, because basically, the stuff that makes virtual functions... well... be virtual hasn't happened yet.

Besides which, clone() normally allocates a new object on the heap. If you implemented the copy constructor in terms of clone(), you'd need to assign the data members back from the allocated object, and then delete that allocated object. And if you were then hoping to implement assignment with the copy-and-swap idiom, well... (oops, copying means using the copy constructor... hello infinite recursion) Basically, you can't really clone "into" an existing object, because that's not what "clone" means. Meanwhile, a constructor can only operate on an already-allocated (whether on the heap or the stack) chunk of memory. It isn't responsible for the actual allocation; it's only responsible for initialization (setup).

The real solution is to realize (which is strongly hinted at by the previous paragraph) that you have *this* part backwards, too. :) That is to say: instead, use the copy constructor to implement clone().

Also note that you can vary the *return type* for such a clone function according to what is actually returned, which gives a bit more type safety.

The result looks like:

class Base {  // other stuff  public:  Base(const Base& b) {    // do whatever's needed here (or better yet, in the initialization list)  }  // Now we implement clone() in terms of the copy ctor:  virtual Base* clone() { return new Base(*this); }};class Derived {  // Similarly:  public:  Derived(const Derived& d) {    // blah blah...  }  Derived* clone() { return new Derived(*this); }}// ...Base* foo = createBaseOrDerived();Base* bar = foo.clone(); // now points at a separate objectbar->action(); // will behave polymorphically, with the same result as foo->action()Derived* baz;Derived* quux = baz.clone(); // no need to cast now


If the 'Derived* clone' bit doesn't work for you, then upgrade your compiler ;) Or as a workaround, just return Base*, and use static_cast when you create quux.

Reference material.

This topic is closed to new replies.

Advertisement