5

In our legacy project we have a function that takes reference to a base class and creates a copy of the derived class on the heap. This is solved essentially like this: https://godbolt.org/z/9ooM4x

#include <iostream>

class Base
{
public:
    virtual Base* vclone() const = 0;
    int a{7};
};

class Derived : public Base
{
public:
    Derived() 
    {
        a = 8; 
    }

    Base* vclone() const override
    {
        return new Derived(*this);
    }
};

Base* clone(const Base& original)
{
    return original.vclone();
}

int main()
{
    Derived d1;;
    auto* d2 = clone(d1);

    std::cout << d2->a << std::endl;
}

This works, but I would like to get rid of the boilerplate vclone method that we have to have in every single derived class.

We have hundreds of derived classes, some of them derived not directly from Base, but from some of the other derived classes too. So if we forget to override the vclone method, we may not even get a warning of the slicing that will happen.

Now, there is much to say about such a design, but this is 10-15 year old code that I try to modernize step by step. What I do look for, is a templatized version of clone that does not depend on a virtual method. What I want, is a clone function like this:

Base* clone(const Base& original)
{
    return new <Actual Derived Type>(original);
}

The actual derived type is somewhat known, since a dynamic_cast will fail if trying to cast to it with wrong type, but I don't know if it is possible to access the actual type in a way that I want. Any help would be appreciated.

4
  • Do you know what the concrete types will be at compile time? Commented Jan 27, 2021 at 9:52
  • 1
    I've no idea how to remove the boiler-plate code. You can minimize it by using a template function for cloning (with type deduction of *this). (Then it can be copy/pasted without the need to edit it.) - At least, I once found a way to ensure that none of the boiler-plate member functions is missing... SO: How to implement ICloneable without inviting future object-slicing Commented Jan 27, 2021 at 9:54
  • Normally clone methods are done through virtual function, since you will need to figure out the type at run-time. If you want a free function like that you will need to manually check what type it is (if, a big switch or some other manner of dispatch). Using the CRTP approach of the downvoted answer seems like a much better approach. Commented Jan 27, 2021 at 10:42
  • see also template cloning Commented Jan 27, 2021 at 11:00

3 Answers 3

2

I also think you probably cannot improve the code in the sense to make it shorter. I would say this implementation is basically the way to go.

What you could do is to change the return value of Derived::clone to Derived *. Yes C++ allows this.

Then a direct use of Derived::clone yields the correct pointer type and Base::clone still works as expected

class Derived : public Base
{
public:
    Derived() 
    {
        a = 8; 
    }

    Derived* vclone() const override  // <<--- 'Derived' instead of 'Base'. 
    {
        return new Derived(*this);
    }
};

I would also rename to vclone member function to clone (There is no need to have two names).

The free function clone could be made a template so that it works for all classes and returns the right pointer type

template <class T>
T *clone(const T *cls)
{
  return cls->clone();
}

However, all these changes do not make the code shorter, just more usable and perhaps more readable.

To make it a little shorter you might use an CRTP approach.

template <class Derived, class Base>
class CloneHelper: public Base {
    Derived* vclone() const override  
    {
        return new Derived(* static_cast<Derived *>(this) );
    }
};
// then use
class Derived : public CloneHelper<Derived, Base>
{
public:
    Derived() 
    {
        a = 8; 
    }
};

However, I am not sure if it is worth it. One still must not forget the CloneHelper, it makes inheritance always public and you cannot delegate to the Base constructor so easily and it is less explicit.

Sign up to request clarification or add additional context in comments.

1 Comment

Thanks. "vclone" is not the real name, just one I used here to indicate that its virtual. The real base class also has a virtual destructor, as it should. I guess I keep it the way the code is now, since any fixes are really minor.
0

You could use an outside clone function and typeid:

#include <typeindex>
#include <string>
#include <stdexcept>
#include <cassert>

template<class Derived_t, class Base_t>
Base_t *clone_helper(Base_t *b) {
    return new Derived_t(*static_cast<Derived_t *>(b));
}

struct Base {
    virtual ~Base() = default;
};

struct Derived : Base {};

Base *clone(Base *b) {
    const auto &type = typeid(*b);
    if (type == typeid(Base)) {
        return clone_helper<Base>(b);
    }
    if (type == typeid(Derived)) {
        return clone_helper<Derived>(b);
    }

    throw std::domain_error(std::string("No cloning provided for type ") + typeid(*b).name());
}

int main() {
    Derived d;
    Base *ptr = &d;

    auto ptr2 = clone(ptr);

    assert(typeid(*ptr2) == typeid(Derived));
}

This will find at runtime if you did not provide a clone method. It may be slower than usual. Sadly a switch is not possible since we cannot obtain the typeid of a type at compile time.

Comments

0

You may like to implement clone function in a separate class template, which is only applied to derived classes when an object of a derived class is created. The derived classes do not implement clone (keep it pure virtual) to avoid forgetting to override it in a further derived class.

Example:

struct Base {
    virtual Base* clone() const = 0;
    virtual ~Base() noexcept = default;
};

template<class Derived>
struct CloneImpl final : Derived {
    using Derived::Derived;

    CloneImpl* clone() const override { // Covariant return type.
        return new CloneImpl(*this);
    }
};

template<class T>
std::unique_ptr<T> clone(T const& t) { // unique_ptr to avoid leaks.
    return std::unique_ptr<T>(t.clone());
}

struct Derived : Base {};
struct Derived2 : Derived {};

int main() {
    CloneImpl<Derived> d1; // Apply CloneImpl to Derived when creating an object.
    auto d2 = clone(d1);
    auto d3 = clone(*d2);

    CloneImpl<Derived2> c1; // Apply CloneImpl to Derived2 when creating an object.
    auto c2 = clone(c1);
    auto c3 = clone(*c2);
}

See https://stackoverflow.com/a/16648036/412080 for more details about implementing interface hierarchies without code duplication.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.