Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Didn't understand Mr. Stroustup's suggestion to delete the copy default and move operations for the abstract class Shape

Tags:

c++

c++11

I'm trying to understand what the author suggests in 3.3.4 Suppressing Operations in his new book (TCPL 4th edition), to no avail.

Excerpt from book

Using the default copy or move for a class in a hierarchy is typically a disaster: Given only a pointer to a base, we simply don’t know what members the derived class has (§3.3.3), so we can’t know how to copy them. So, the best thing to do is usually to delete the default copy and move operations; that is, to eliminate to default definitions of those two operations:

class Shape {
public:
    Shape(const Shape&) =delete; // no copy operations
    Shape& operator=(const Shape&) =delete;
    Shape(Shape&&) =delete; //no move operations
    Shape& operator=(Shape&&) =delete;
    ~Shape();
};

Now an attempt to copy a Shape will be caught by the compiler. If you need to copy an object in a class hierarchy, write some kind of clone function (§22.2.4).

For example, the code below doesn't compile with Shape(const Shape&) = delete;, as the clone() function invokes Shape's copy constructor.

#include <iostream>

class Shape
{
    public:
    virtual ~Shape() {}
    Shape() {}
    Shape(const Shape&) {};
    virtual Shape* clone() const = 0;
};

class Circle: public Shape
{
    public:
    Circle(int i) : a(i) {}
    Circle* clone() const { return new Circle(*this); }
    int a;
};

int main()
{
    Shape* p = new Circle(1);
    Shape* q = p->clone();

    std::cout << dynamic_cast<Circle*>(p)->a << std::endl;
    std::cout << dynamic_cast<Circle*>(q)->a << std::endl; 

}    
like image 601
Wake up Brazil Avatar asked Jul 28 '13 21:07

Wake up Brazil


2 Answers

If you only have a pointer to a Shape, then you can't possibly make a copy of the actual implementation - it will (most likely) be larger, so your copy will be "sliced". In your example, Circle will have an extra int a; that isn't part of the Shape class - which would be lost if you just plain copy a Shape class object without knowing it's a Circle (and the whole point of polymorphism is that you are not supposed to "know" what object is what type when dealing with it in generic functions)

To avoid having problems caused by accidentally using something like:

*q = *p; 

it is better to "delete" the operators that will allow you to do that

However, as the copy constructor is needed for the case you describe, so one solution is to make it protected - that protects against something other than a derived class using it, and works correctly.

Thanks to robson below (and a nights sleep as well), the solution is clearly to make a copy-constructor in Circle. Just because you don't have one for Shape doesn't mean that you can't have one in the derived class:

class Circle: public Shape
{
    public:
    Circle(int i) : a(i) {}
    Circle(const Circle& other) { a = other.a; }    // Note this line!
    Circle* clone() const { return new Circle(*this); }
    int a;
};

The reason it tries to use the Shape copy constructor is because you haven't got one in your own class. You should!

You could also do (as Robson explained):

class Circle: public Shape
{
    public:
    Circle(int i) : a(i) {}
    Circle* clone() const { return new Circle(a); }
    int a;
};

And not need a copy constructor at all. Both of these solutions solve the "you are trying to use a deleted Shape(const Shape &) constructor. It really is obvious once you see it.

like image 52
Mats Petersson Avatar answered Oct 05 '22 23:10

Mats Petersson


He meant that its bad to make them accessible from outside, due to potentioal object slicing problems. It you dont need to make the class clonable than deleting is sufficient, otherwise you can just make it protected to be accessible only within clone() and successors.

like image 36
yuri kilochek Avatar answered Oct 06 '22 01:10

yuri kilochek