views:

346

answers:

3

I want to perform "deep copies" of an STL container of pointers to polymorphic classes.

I know about the Prototype design pattern, implemented by means of the Virtual Ctor Idiom, as explained in the C++ FAQ Lite, Item 20.8.
It is simple and straightforward:

struct ABC // Abstract Base Class
{
    virtual ~ABC() {}
    virtual ABC * clone() = 0;
};
struct D1 : public ABC
{
    virtual D1 * clone() { return new D1( *this ); } // Covariant Return Type
};

A deep copy is then:

for( i = 0; i < oldVector.size(); ++i )
    newVector.push_back( oldVector[i]->clone() );

Drawbacks

As Andrei Alexandrescu states it:

The clone() implementation must follow the same pattern in all derived classes; in spite of its repetitive structure, there is no reasonable way to automate defining the clone() member function (beyond macros, that is).

Moreover, clients of ABC can possibly do something bad. (I mean, nothing prevents clients to do something bad, so, it will happen.)

Better design?

My question is: is there another way to make an abstract base class clonable without requiring derived classes to write clone-related code? (Helper class? Templates?)


Following is my context. Hopefully, it will help understanding my question.

I am designing a class hierarchy to perform operations on a class Image:

struct ImgOp
{
    virtual ~ImgOp() {}
    bool run( Image & ) = 0;
};

Image operations are user-defined: clients of the class hierarchy will implement their own classes derived from ImgOp:

struct CheckImageSize : public ImgOp
{
    std::size_t w, h;
    bool run( Image &i ) { return w==i.width() && h==i.height(); }
};
struct CheckImageResolution { ... };
struct RotateImage          { ... };
...

Multiple operations can be performed sequentially on an image:

bool do_operations( vector< ImgOp* > v, Image &i )
{
    for_each( v.begin(), v.end(),
        /* bind2nd( mem_fun( &ImgOp::run ), i ... ) don't remember syntax */ );
}

If there are multiple images, the set can be split and shared over several threads. To ensure "thread-safety", each thread must have its own copy of all operation objects contained in v -- v becomes a prototype to be deep copied in each thread.

Edited: The thread-safe version uses the Prototype design pattern to enforce copy of pointed-to-objects -- not ptrs:

struct ImgOp
{
    virtual ~ImgOp() {}
    bool run( Image & ) = 0;
    virtual ImgOp * clone() = 0; // virtual ctor
};

struct CheckImageSize : public ImgOp       { /* no clone code */ };
struct CheckImageResolution : public ImgOp { /* no clone code */ };
struct RotateImage : public ImgOp          { /* no clone code */ };

bool do_operations( vector< ImgOp* > v, Image &i )
{
    // In another thread
    vector< ImgOp* > v2;
    transform( v.begin(), v.end(),                       // Copy pointed-to-
        back_inserter( v2 ), mem_fun( &ImgOp::clone ) ); // objects
    for_each( v.begin(), v.end(),
        /* bind2nd( mem_fun( &ImgOp::run ), i ... ) don't remember syntax */ );
}

This has sense when image operation classes are small: do not serialize accesses to unique instances of ImgOps, rather provide each thread with their own copies.

The hard part is to avoid writers of new ImgOp-derived classes to write any clone-related code. (Because this is implementation detail -- this is why I dismissed Paul's answers with the Curiously Recurring Pattern.)

+6  A: 

You can use the curiously recursive pattern but it might make your code less readable. You will still need copy constructors. It works as follows.

struct ABC // Abstract Base Class
{
    virtual ~ABC() {}
    virtual ABC * clone() const = 0;
};



template <class TCopyableClass>
struct ClonableABC : public ABC
{
    virtual ABC* clone() const {
       return new TCopyableClass( *(TCopyableClass*)this );
    } 
};


struct SomeABCImpl : public ClonableABC<SomeABCImpl>
{};
poulejapon
+1: Always interested in a curriously recursive pattern application ...
neuro
+1 good approach. As pointed out in the article Tyler mentioned in a comment, you no longer have a covariant return type. I never understood why that was such a big deal though. The places I'd call a clone method, I don't know the derived type anyway.
Dan
As I understand this won't work if I need to subclass `SomeABCImpl`, right?
doublep
I thought about this solution but dismissed it because client still has to write some clone-related code: the template parameter of `ClonableABC`. From the perspective of the client, the intent of this template parameter is not clear and might seem useless.
Julien L.
doublep > Indeed. Not with this piece of code.Julien L. > I understand your choice. This might be somewhat misleading for the client.
poulejapon
+1  A: 

A deep copy is then: [for loop]

You make the client clone the vector explicitly. I'm not sure if this answers your question, but I would suggest a vector of smart pointers so the cloning happens automatically.

std::vector<cloning_pointer<Base> > vec;
vec.push_back(cloning_pointer<Base>(new Derived()));

// objects are automatically cloned:
std::vector<cloning_pointer<Base> > vec2 = vec;

Of course, you don't want these implicit copies to happen when resizing a vector or something, so you need to be able to distinguish copies from moves. Here is my C++0x toy implementation of cloning_pointer which you might have to adjust to your needs.

#include <algorithm>

template<class T>
class cloning_pointer
{
    T* p;

public:

    explicit cloning_pointer(T* p)
    {
        this->p = p;
    }

    ~cloning_pointer()
    {
        delete p;
    }

    cloning_pointer(const cloning_pointer& that)
    {
        p = that->clone();
    }

    cloning_pointer(cloning_pointer&& that)
    {
        p = that.p;
        that.p = 0;
    }

    cloning_pointer& operator=(const cloning_pointer& that)
    {
        T* q = that->clone();
        delete p;
        p = q;
        return *this;
    }

    cloning_pointer& operator=(cloning_pointer&& that)
    {
        std::swap(p, that.p);
        return *this;
    }

    T* operator->() const
    {
        return p;
    }

    T& operator*() const
    {
        return *p;
    }
};

Julien: && is not a "ref of ref", it is an rvalue reference which only binds to modifiable rvalues. See this excellent (but sadly slightly outdated) tutorial and video for an overview of rvalue references and how they work.

FredOverflow
This is interesting. Please, can you give more details on the "moving" mechanism: How does the cloning_pointer know it must move and not clone? Each time it is given a ref of ref, it does mean it is being resized?Btw, you should not use `std::swap()` but `using std::swap; swap().
Julien L.
@Julien I know about the swap idiom, but I think it's pretty safe to swap two raw pointers with `std::swap` ;-) I included two links for you that explain the moving mechanism quite well.
FredOverflow
Thanks a lot for the details (and for the swap correction :) I like the helper class `cloning_pointer ` because it makes the intent clear. However, it does not solve my problem because derived classes still have to implement the `clone()` method.
Julien L.
@Julien You're welcome. Concerning swap, it wasn't really a "correction" on my part... I guess I was just being lazy :-) Generally, the idiom you described should definitely be preferred.
FredOverflow
+1  A: 

Hi, FYI, this is the design I came out with. Thank you Paul and FredOverflow for your inputs. (And Martin York for your comment.)

Step #1, Compile-time polymorphism with templates

Polymorphism is performed at compile-time using templates and implicit-interfaces:

template< typename T >
class ImgOp
{
    T m_t; // Not a ptr: when ImgOp is copied, copy ctor and
           // assignement operator perform a *real* copy of object
    ImageOp ( const ImageOp &other ) : m_t( other .m_t ) {}
    ImageOp & operator=( const ImageOp & );
public:
    ImageOp ( const T &p_t ) : m_t( p_t ) {}
    ImageOp<T> * clone() const { return new ImageOp<T>( *this ); }
    bool run( Image &i ) const { return m_t.run( i); }
};

// Image operations need not to derive from a base class: they must provide
// a compatible interface
class CheckImageSize       { bool run( Image &i ) const {...} };
class CheckImageResolution { bool run( Image &i ) const {...} };
class RotateImage          { bool run( Image &i ) const {...} };

Now all the clone-related code lies within a unique class. However, it is now impossible to have a container of ImgOps templatized on different operations:

vector< ImgOp > v;           // Compile error, ImgOp is not a type
vector< ImgOp< ImgOp1 > > v; // Only one type of operation :/

Step #2, Add a level of abstraction

Add a non-template base acting as an interface:

class AbstractImgOp
{
    ImageOp<T> * clone() const = 0;
    bool run( Image &i ) const = 0;
};

template< typename T >
class ImgOp : public AbstractImgOp
{
    // No modification, especially on the clone() method thanks to
    // the Covariant Return Type mechanism
};

Now we can write:

vector< AbstractImgOp* > v;

But it becomes hard to manipulate image operation objects:

AbstractImgOp *op1 = new AbstractImgOp;
    op1->w = ...; // Compile error, AbstractImgOp does not have
    op2->h = ...; // member named 'w' or 'h'

CheckImageSize *op1 = new CheckImageSize;
    op1->w = ...; // Fine
    op1->h = ...;
AbstractImgOp *op1Ptr = op1; // Compile error, CheckImageSize does not derive
                             // from AbstractImgOp? Confusing

CheckImageSize op1;
    op1.w = ...; // Fine
    op1.h = ...;
CheckImageResolution op2;
    // ...
v.push_back( new ImgOp< CheckImageSize >( op1 ) );       // Confusing!
v.push_back( new ImgOp< CheckImageResolution >( op2 ) ); // Argh

Step #3, Add a "cloning pointer" class

Based on the FredOverflow's solution, make a cloning pointer to make the framework simpler to use.
However, this pointer needs not to be templatized for it is designed to hold only one type of ptr -- only the ctor needs to be templatized:

class ImgOpCloner
{
    AbstractImgOp *ptr; // Ptr is mandatory to achieve polymorphic behavior
    ImgOpCloner & operator=( const ImgOpCloner & );
public:
    template< typename T >
    ImgOpCloner( const T &t ) : ptr( new ImgOp< T >( t ) ) {}
    ImgOpCloner( const AbstractImgOp &other ) : ptr( other.ptr->clone() ) {}
    ~ImgOpCloner() { delete ptr; }
    AbstractImgOp * operator->() { return ptr; }
    AbstractImgOp & operator*() { return *ptr; }
};

Now we can write:

CheckImageSize op1;
    op1.w = ...; // Fine
    op1.h = ...;
CheckImageResolution op2;
    // ...
vector< ImgOpCloner > v;
v.push_back( ImgOpCloner( op1 ) ); // This looks like a smart-ptr, this is not
v.push_back( ImgOpCloner( op2 ) ); // confusing anymore -- and intent is clear
Julien L.