How to fix a copy constructor that doesn't work?

Question

I'm trying to simplify my code. Why I can't use the commented constructor instead of the previous (not commented) one?

struct Direction {
    const double x, y, z;
    Direction(double _X, double _Y, double _Z) : x(_X), y(_Y), z(_Z) {}
    Direction(Direction& _D) : x(_D.x), y(_D.y), z(_D.z) {}
}

class Movement {
private:
    const double v;
    const Direction d;
public:
    Movement(double _V, double _X, double _Y, double _Z) : v(_V), d(_X, _Y, _Z) {}
    Movement(double _V, Direction _D) : v(_V), d(_D) {}
    Movement(Movement& _M) : v(_M.v), d(_M.d.x, _M.d.y, _M.d.z) {} // this works
 // Movement(Movement& _M) : v(_M.v), d(_M.d) {}                   // this doesn't work
}

Any ideas on how to make it work? Thank you.

`_M.d` is `Direction const` while `Direction`'s copy constructor takes a non-`const` l-value reference. How can it compile? Why does `Direction`'s copy constructor take a non-`const` l-value reference? Why do you have `const` private members? — paolo, Aug 12 '22 at 10:26
Typo: https://godbolt.org/z/qq814jWGe - copy constructor needs `const`. — Marek R, Aug 12 '22 at 10:34
[Dupe1](https://stackoverflow.com/questions/27812119/binding-of-reference-to-a-value-of-type-drops-qualifiers) — Jason, Aug 12 '22 at 11:56

463035818_is_not_an_ai · Accepted Answer · 2022-08-12T11:46:11.660

The error message is rather clear:

<source>: In copy constructor 'Movement::Movement(Movement&)':
<source>:15:44: error: binding reference of type 'Direction&' to 'const Direction' discards qualifiers
   15 |     Movement(Movement& _M) : v(_M.v), d(_M.d) {}                   // this doesn't work
      |                                         ~~~^
<source>:4:26: note:   initializing argument 1 of 'Direction::Direction(Direction&)'
    4 |     Direction(Direction& _D) : x(_D.x), y(_D.y), z(_D.z) {}
      |               ~~~~~~~~~~~^~

The member d is const, but the Direction constructor expects a non-const reference.

The code compiles if the constructor takes a constant referece, which it should, because the constructor does not modify the parameter:

struct Direction {
    const double x, y, z;
    Direction(double _X, double _Y, double _Z) : x(_X), y(_Y), z(_Z) {}
    Direction(const Direction& _D) : x(_D.x), y(_D.y), z(_D.z) {}
};

class Movement {
private:
    const double v;
    const Direction d;
public:
    Movement(double _V, double _X, double _Y, double _Z) : v(_V), d(_X, _Y, _Z) {}
    Movement(double _V, Direction _D) : v(_V), d(_D) {}
    //Movement(Movement& _M) : v(_M.v), d(_M.d.x, _M.d.y, _M.d.z) {} // this works
    Movement(Movement& _M) : v(_M.v), d(_M.d) {}                   // this doesn't work
};

int main() {
    Movement x(0.1,0.2,0.3,0.4);
    Movement y(x);
}

However, const members are tricky and rarely the right way. The members are private, so there is no point in making them const even on a non-const Movement. The user has no way to modify them anyhow. I suggest to remove all const from members and add const to all reference arguments that can be const. Especially the copy constructor should take its parameter as const &.

struct Direction {
    double x, y, z;
    Direction(double _X, double _Y, double _Z) : x(_X), y(_Y), z(_Z) {}
    Direction(const Direction& _D) : x(_D.x), y(_D.y), z(_D.z) {}
};

class Movement {
private:
    double v;
    Direction d;
public:
    Movement(double _V, double _X, double _Y, double _Z) : v(_V), d(_X, _Y, _Z) {}
    Movement(double _V, const Direction& _D) : v(_V), d(_D) {}
    Movement(const Movement& _M) : v(_M.v), d(_M.d) {}                   // this doesn't work
};

int main() {
    Movement x(0.1,0.2,0.3,0.4);
    Movement y(x);
}

If Directions members should also not be modifiable after construction make them private. Making a class member private is sufficient to prevents its modification, while const members prevent a couple of useful operations (assignment etc).

How to fix a copy constructor that doesn't work?

1 Answers1