Implementing copy constructor for polymorphic class

Question

Suppose I'm dealing with the following header file(for experimental purposes):

class Base{
protected: 
    Data& data1;
public:
    Base(Data& data1);
    virtual Base* clone() const =0;
    virtual ~Base();
    virtual void do() = 0;
};
class Derived: public Base{
private:
     const int data2;
public:
     Derived(int data2, Data& data1);
     virtual Derived* clone() const;
     ~Derived(); 
}

I tried to implement these classes which are fairly simple:

Base:

Base::Base(Data& data1): data1{data1} {}
virtual Base::~Base() = default;

Derived: 

Derived::Derived(int data2, Data& data1): Base(data1), data2{data2} {}
virtual Derived* Derived::clone() const {
    return new Derived(*this);
}
virtual void Derived::do() {
    return;
}

So far so good, but I'm having a couple of issues:

Say I wanted to implement a copy constructor and a copy assignment operator for this hierarchy, should I do it in both classes? If so, is this a good way to do it? :

Base: 

Base::Base(const Base& other): Base(other.data1) {}
Base& Base::operator=(const Base& rhs){
    if(this == &rhs)
        return *this;
    data1 = rhs.data1; // is this possible? since data1 is a reference (cannot be reassigned)
    return *this;
}

and which is a proper way to do so for derived?:

Derived:

Derived::Derived(const Derived& other): Base(other.data1), data2{other.data2} {}
OR
Derived::Derived(const Derived& other): Derived(other.data2, other.data2) {}

Derived& Derived::operator=(const Derived& rhs){
    if(this == &rhs)
        return *this;
    data1 = rhs.data1;
    data2 = rhs.data2; // data2 is a const variable so this is probably invalid too 
    return *this;
}

Once I implement a copy constructor for derived, I get an 'Endless loop' warning when returning from clone() method. Why is that?
Is there a proper way to implement rule of 3/ rule of 5 for classes of such hierarchy?

Thanks ahead.

I highly recommend changing your coding guidelines so that member variables have a different naming scheme than method parameters. — Thomas Matthews
– Thomas Matthews, Commented Nov 12, 2020 at 19:05
A reference member and const members of a class cannot be reassigned, so are generally only able to be initialised in a constructor initialiser list. If a class has such members (or a base with such members) there may be a case for explicitly NOT having an assignment operator and/or not having a copy constructor. — Peter
– Peter, Commented Nov 12, 2020 at 19:08
@Peter I get why copy assignment is problematic, but why is it not okay to have a copy constructor? and why would that cause an infinite loop? — giorgioh
– giorgioh, Commented Nov 12, 2020 at 19:09
but why is it not okay to have a copy constructor? -- The question you should ask yourself is this -- does it make logical sense to have copies of Base? There are some situations where it doesn't logically make sense to copy an object, for example the istream and ostream's are not copyable. — PaulMcKenzie
– PaulMcKenzie, Commented Nov 12, 2020 at 19:18
@PaulMcKenzie in this case it's not needed since there aren't any raw pointers involved. But generally, this is an abstract class that does have data members. If I did wanna copy one of its derived classes, I assume I would need a copy constructor for it as well, wouldn't I? — giorgioh
– giorgioh, Commented Nov 12, 2020 at 19:31

Jarod42 · Accepted Answer · 2020-11-12 21:35:16Z

1

Say I wanted to implement a copy constructor and a copy assignment operator for this hierarchy, should I do it in both classes? If so, is this a good way to do it?

Yes, you have to provide copy constructor/assignment.

Best way, when possible/correct, is to use default.

For assignment, your issue is unrelated to hierarchy, but related to member you use:

Reference cannot be rebound. const member cannot be changed.

So default assignment cannot be generated, mark them as deleted.

class Base
{
// ...
    Base(const Base&) = default;
    Base& operator=(const Base&) = delete;
};

class Derived
{
// ...
    Derived(const Derived&) = default;
    Derived& operator=(const Derived&) = delete;
};

Once I implement a copy constructor for derived, I get an 'Endless loop' warning when returning from clone() method. Why is that?

Cannot reproduce from comment

Is there a proper way to implement rule of 3/ rule of 5 for classes of such hierarchy?

Hierarchy is unrelated to rule of 5/3/0.

Just respect rule of 5/3/0 for each class.

answered Nov 12, 2020 at 21:35

Jarod42

228k15 gold badges215 silver badges355 bronze badges

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

1 Comment

giorgioh Over a year ago

apparently that endless loop thing is a Clion bug. Regardless of that, I thought I had my head wrapped around when to use copy & move constructors\ assignment operators, but apparently I don't. Could you please clarify? Does it make sense to have an abstract base class, which also has a virtual clone method to also have a copy/move constructor/ assignment operator in it? Or maybe does it even make sense to have these if there's no clone method? If so, what level of visibility is appropriate? private/ protected? )Assuming it does have dynamic data allocated) Thanks.

Collectives™ on Stack Overflow

Implementing copy constructor for polymorphic class

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related