5

According to CppCoreGuideline, I should disable the copy constructor of a base class and propose a clone method: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-copy-virtual

For example:

class B {
   public:
     explicit B() = default;
     B(B&&) = default; // Move constructor
     B& operator=(B&&) = default; // Move assignment operator
     B(const B&) = delete; // Copy constructor
     B& operator=(const B&) = delete; // Copy assignment
     virtual ~B() = default;

     virtual unique_ptr<B> clone()
     {
        return unique_ptr<B>{new B{*this}}; // how do this without copy constructor ?
     }
    private:
     int c;
     int d;
};

class D : public B {
    public:
     explicit D() = default;
     D(D&&) = default; // Move constructor
     D& operator=(D&&) = default; // Move assignment operator
     D(const B&) = delete; // Copy constructor
     D& operator=(const D&) = delete; // Copy assignment
     virtual ~D() = default;

     virtual unique_ptr<B> clone() override
     {
          //  how can I copy all private data member of base class ???
     }
};

but how can I copy all private data member in clone method? Obviously I'll use the CRTP pattern : C++: Deep copying a Base class pointer

Joseph Garnier
  • 117
  • 1
  • 10
  • instead of declaring base's virtual `clone` function as pure, maybe implement it and when overriding it in derived classes, call the base version additionally? – Fureeish Oct 24 '17 at 19:47
  • Move constructor or a method? – Vivick Oct 24 '17 at 19:47
  • @Charles link what code? OP linked the guide and jumped to specific section, which does not contain only the code, but additionally the explanation and information. The fact that OP should/could copy and emphasise specific parts could be debatable. Do not understand the downvote, unless there is a duplicate – Fureeish Oct 24 '17 at 19:56
  • @Fureeish whoops. Saw a github link. – Charles Oct 24 '17 at 19:58
  • I edited my question. Thanks for your interest. – Joseph Garnier Oct 24 '17 at 20:03
  • "This document is an early draft. It's known to be incomplet, incorrekt, and has lots of bad formatting." – zdf Oct 24 '17 at 20:10

2 Answers2

4

I think the simplest way is to actually make the special members protected instead of deleted. This still prevents slicing, but makes it easier to implement clone(). Note that both the copy and move members need to be treated this way.

class B {
public:
    // if this is truly intended to be a polymorphic base class, it probably
    // doesn't make sense for the base to be able to clone itself.
    virtual unique_ptr<B> clone() = 0;

protected:
    B(B const& ) = default;
    B& operator=(B const& ) = default;

private:
    int c;
    int d;
};

Which also allows the derived classes to do this easily:

class D : public B {
public:
    D(D const& ) = default; // this is safe now
    D& operator=(D const& ) = default;

    unique_ptr<B> clone() override {
        return unique_ptr<D>(new D(*this));
    }
    // ...
};
Barry
  • 286,269
  • 29
  • 621
  • 977
  • Out of curiosity, is there any reason to keep the assignment operator around? You'd only need that if you were directly assigning objects of these types to one another, which I can't imagine being doing type-safely if the objects are handled through pointers. – templatetypedef Oct 24 '17 at 20:08
  • @templatetypedef Depends I guess. Might be safe to assign a `D`, might not be. – Barry Oct 24 '17 at 20:09
  • clone() has to be public if you want to copy a D but only have its A* – Michaël Roy Oct 24 '17 at 20:48
  • @MichaëlRoy Don't understand. `clone()` *is* public? – Barry Oct 24 '17 at 21:30
  • `A::clone()` has to be public and not private. That's idea behind making `clone` virtual. – Michaël Roy Oct 25 '17 at 02:47
  • @MichaëlRoy Again, I don't understand the comment. I didn't make `clone()` private. – Barry Oct 25 '17 at 12:16
3

Rather than disabling the copy constructor, consider marking it protected. That way, clients of the class can't accidentally create a copy, but instances of the class can invoke the copy constructor as needed to implement the clone function. You can use the defaulted version of the copy constructor assuming you aren't doing any explicit resource management. Then, to implement clone, you can do something like this:

virtual unique_ptr<B> clone() override
{
    return make_unique<D>(*this);
}

This invokes the object's own (protected) copy constructor, which in turn will invoke the base's (protected) copy constructor, etc.

As a note, there's no need to use CRTP here. Using good old fashioned copy constructors should be all you need.

templatetypedef
  • 362,284
  • 104
  • 897
  • 1,065