Using *this as const in copy constructor to clone self

Question

I am trying to implement a prototype pattern.When I pass self object with *this in order to clone itself with copy constructor I cannot access self member functions due to:

error: passing ‘const ConcreteClonable1’ as ‘this’ argument discards qualifiers [-fpermissive]

The error has to do with wrong usage of const. However, if I remove const from copy constructor everything works. I want to use copy constructor as it is supposed to, with const argument, and the ability to access non-const class members.

Here is the code:

/* Prototype base class. */
class Prototype
{
    protected:
        std::string type;
        int value;
    public:
        virtual Prototype* clone() = 0;
        std::string getType() { return type; }
        int getValue() { return value; }
};

//clonable class
class ConcreteClonable1 : public Prototype
{
    public:
        ConcreteClonable1(int number)
        {
            std::cout << "ConcreteClonable1 cnstr\n";
            type  = "Type1";
            value = number;
        }
        //compilation error if const is used
        ConcreteClonable1 (const ConcreteClonable1& x)
        {
            std::cout << "ConcreteClonable1 copy cnstr\n";
            type  = x.getType();
            value = x.getValue();
        }
        Prototype* clone() { return new ConcreteClonable1(*this); }
};

The objects are initialized in a Factory.

The question is why is this happening? Is there a better way to do this with some kind of copy function from c++ STL?

as a rule of thumb you can make all your methods `const` and only if you need to modify `this` remove it — 463035818_is_not_an_ai, Feb 07 '18 at 12:34
This question is not really about the Prototype pattern, it's just "why can't I call these member functions on a const object?" which has been asked a thousand times on StackOverflow already, and the answer is because you didn't make the member functions `const`. — Jonathan Wakely, Feb 07 '18 at 12:49
@JonathanWakely it should also provoke the question "why *am* I calling these member functions *at all*"? — Caleth, Feb 07 '18 at 12:51
@Caleth or even better "why did I write getters in the first place?" — 463035818_is_not_an_ai, Feb 07 '18 at 12:52
@user463035818 These functions could be something else more complex. The example here is for simplicity... — BugShotGG, Feb 07 '18 at 13:02

Bathsheba · Answer 1 · 2018-02-07T13:05:34.153

2

Your const correctness is all over the place. The immediate problem is that getType and getValue need to be marked const else they cannot be used if the implicit this pointer is const, which it is in your case.

Along with this, you should also fix your clone() function to be const too (although this is not required for your immediate compilation issue):

Prototype* clone() const { return new ConcreteClonable1(*this); }

Cloning after all should not modify the current object.

edited Feb 07 '18 at 13:05

answered Feb 07 '18 at 12:33

Bathsheba

231,907
34
361
483

I tried that and I get the same error – BugShotGG Feb 07 '18 at 12:36
1

that's not the problem. the problem is that he uses non-const member functions in the copy ctor – Sopel Feb 07 '18 at 12:36
@BugShotGG: You *must* change the "get" functions too. – Bathsheba Feb 07 '18 at 12:38
@Sopel et al.: If you don't need to mark the `clone` function `const` then please downvote this - I'm in a meeting suffering from a mental block. – Bathsheba Feb 07 '18 at 12:39
@Sopel et al. Fixed it. Yes, you're all correct. – Bathsheba Feb 07 '18 at 13:06
2

@Bathsheba While clone doesn't need to be const; neither do a lot of functions. I would consider correcting it as part of the fix – UKMonkey Feb 07 '18 at 13:06
1

@UKMonkey Oh for a time machine, so that `mutable` can be the modifier, and `const` the default – Caleth Feb 07 '18 at 13:07
1

@Caleth: Absolutely. And remove `std::vector` and perhaps even `bool` too. And `class` for that matter. And the fact that `'c'` is a `char` type, rather than an `int`. – Bathsheba Feb 07 '18 at 13:08
@Caleth a number of people on SO (including Bathsheba I believe) have asked if I'm from the future .... so I code how I think the standard should be rather than how it is and then 'adjust' as required. Sue me. But agreed; I wish they'd stop putting useless things in the standard and work on features like that! – UKMonkey Feb 07 '18 at 13:17
@UKMonkey: Can you let me know the outcome of the case before I consider suing you? – Bathsheba Feb 07 '18 at 13:18
2

@Bathsheba Chuck Norris wins – UKMonkey Feb 07 '18 at 13:18

score 2 · Answer 2 · answered Feb 07 '18 at 12:38

2

You're trying to call non-const member functions of Prototype on the const object of ConcreteClonable1 : public Prototype in the ConcreteClonable1's copy constructor. To make it work you have to make them const (or not use them at all, se below).

Apart from that you don't really need to know about Prototype to copy ConcreteClonable1. Just copy the members.

answered Feb 07 '18 at 12:38

Sopel

1,179
1
10
15

Thanks, making `getType` and `getValue` `const` worked. I now understand why that happened. Completely forgot the access rule of `const` and `nonconst` – BugShotGG Feb 07 '18 at 12:42

Caleth · Accepted Answer · 2018-02-07T12:49:09.203

2

Just delegate the copy construction of Prototype

ConcreteClonable1::ConcreteClonable1(const ConcreteClonable1 & x) : Prototype(x) 
{ std::cout << "ConcreteClonable1 copy cnstr\n"; }

In general you should prefer member initialisation over assigning in the body of the constructor.

Prototype::Prototype(std::string type_, int value_) : type(type_), value(value_) {}

ConcreteClonable1::ConcreteClonable1(int number) : Prototype("Type1", number) 
{ std::cout << "ConcreteClonable1 cnstr\n"; }

edited Feb 07 '18 at 12:49

answered Feb 07 '18 at 12:42

Caleth

52,200
2
44
75

yup, that's better – Sopel Feb 07 '18 at 12:44
@Caleth Thanks for your answer. I have a question. Why `: Prototype(x)` works without the need to declare `getType` and `getValue` as `const` ? – BugShotGG Feb 07 '18 at 12:53
because there is a compiler generated `Prototype::Prototype(const Prototype &)` copy constructor that copies the data members directly, and doesn't involve `getType` or `getValue` – Caleth Feb 07 '18 at 12:55
Which is why `struct pair { std::string first; int second; };` is a usable type, e.g. `pair one = { "just works", 1 }, two; two = one;` – Caleth Feb 07 '18 at 12:57
@Caleth Using `: type(x.getType()), value(x.getValue())` returns an error `does not have any field named ‘value’`. I guess its because of abstract class `Prototype`? – BugShotGG Feb 07 '18 at 13:00
You can *simply omit* ConcreteClonable1's copy constructor, and it will *just work*, albeit without any place for the `std::cout << "ConcreteClonable1 copy cnstr\n";` to go – Caleth Feb 07 '18 at 13:00
`type` and `value` are initialised by the construction of the `Prototype` base object. The rules of constructors say you only get *one* place to initialise the data members, you *can't* do it a **second time** in a derived initialiser list – Caleth Feb 07 '18 at 13:02
1

There is a call to `Prototype::Prototype()`, which occurs at the start of the member initialisation of `ConcreteClonable1`. it's nothing to do with the abstractness of `Prototype`, merely the fact that *it* has the members *directly*, `ConcreteClonable1` only has them *indirectly* through `Prototype` – Caleth Feb 07 '18 at 13:05
@Caleth Thanks for clarifying that. – BugShotGG Feb 07 '18 at 13:30

Using *this as const in copy constructor to clone self

3 Answers3