1

Consider this code. I have defined a copy constructor for Person that will copy all data except for one, namely its DontCopy data member. But I can't seem to get the same thing done with the assignment operator. What is the best way to do it apart from what I've done already below? Currently, I've done it by manually setting the name member only. What if there are hundreds of members of Person that need to be copied, with only one that is not to be copied? The copy constructor takes care of this problem, but the assignment operator does not.

#include <iostream>
#define show(variable) std::cout << #variable << " = " << variable << '\n';

struct DontCopy {
    class Person& subject;
    DontCopy (Person& p) : subject(p) { }
};

class PersonDataDoCopy {
protected:
    std::string name;
    PersonDataDoCopy (const std::string& n) : name(n) { }
    PersonDataDoCopy() = default;
};

class Person : private PersonDataDoCopy {
    DontCopy dontCopy{*this};
public:
    Person (const std::string& n) : PersonDataDoCopy(n) { }
    Person (const Person& other) : PersonDataDoCopy(other) { }  // 'dontCopy' is not copied from 'other'.
    Person (Person&& other) : PersonDataDoCopy(std::move(other)) { }  // 'dontCopy' is not copied from 'other'.
    Person& operator= (const Person& other) {
        if (this == &other)
            return *this;
//      static_cast<PersonDataDoCopy>(*this) = PersonDataDoCopy(other);  // Does not copy data from PersonDataDoCopy
//      *this = static_cast<Person&>(PersonDataDoCopy(other));  // Does not compile.
        name = other.name;  // Need more general solution than this.
        return *this;
    }
    Person() = default;
    void display() const { show(name)  show(this)  show(&dontCopy.subject); }
};

int main() {
    Person bob("Bob");
    bob.display();
    std::cout << '\n';
    Person clone1(bob);
    clone1.display();
    std::cout << '\n';
    Person clone2("");
    clone2 = bob;
    clone2.display();
}

Output:

name = Bob
this = 0x6ffde0
&dontCopy.subject = 0x6ffde0

name = Bob
this = 0x6ffdd0
&dontCopy.subject = 0x6ffdd0

name = Bob
this = 0x6ffdc0
&dontCopy.subject = 0x6ffdc0
prestokeys
  • 4,817
  • 3
  • 20
  • 43
  • What is the issue? Could you describe it in more details instead of *I can't seem to get the same thing done* – 273K Jan 29 '23 at 23:14
  • What does "Does not copy data from PersonDataDoCopy" mean, in the first comment? It is true that this is not 100% right, but a copy should still take place, despite its irrelevant trip to the countyside. – Sam Varshavchik Jan 29 '23 at 23:15
  • 2
    What happened when you tried? Try expressing your problem in terms of "expected results" vs "actual results" – Wyck Jan 29 '23 at 23:15
  • 3
    Maybe you want to write `PersonDataDoCopy::operator=(other);` ? Just like this, it's a complete C++ statement. – yeputons Jan 29 '23 at 23:16
  • I've updated my intro to help you understand the issue better. – prestokeys Jan 29 '23 at 23:17
  • FWIW you'll want to learn about the [copy/swap idiom](https://stackoverflow.com/q/3279543/845568). – Captain Obvlious Jan 29 '23 at 23:18
  • @ yeputons. Yes I think that is the line I was trying to figure out. – prestokeys Jan 29 '23 at 23:18
  • @prestokeys *I have defined a copy constructor for Person that will copy all data except for one, namely its DontCopy data member* -- And on purpose, you have walked yourself directly into one of the hardest bugs to track down. That bug being making partial copies of an object. The copy/assignment operators should be written with no hidden side-effects, since you don't know or cannot predict when or where those copies will be made. If you want the ability to make parital copies, create a named function -- don't use the copy/assignment operators to do this. – PaulMcKenzie Jan 29 '23 at 23:24
  • But what if I'm certain that the `DontCopy` data member must never, ever be copied? Its `subject` data member suggests why it must never be copied by a different `Person`, as it must always refer to the `Person` that owns it, not to the one being copied from. – prestokeys Jan 29 '23 at 23:37
  • Then completely turn off copy and assignment, and have a `clone()` or similar named function. Then copying and assingning is completely under your control. – PaulMcKenzie Jan 29 '23 at 23:43
  • Adding `DontCopy(const DontCopy & rhs) = delete;` and `DontCopy & operator =(const DontCopy & rhs) = delete;` to your `DontCopy` class will tell the compiler to make any attempts to copy a `DontCopy` into a compile-time error, which sounds like what you want. (On older C++ compilers that don't support this use of the `delete` keyword, you could just make those declarations private instead) – Jeremy Friesner Jan 29 '23 at 23:45

1 Answers1

0

If you don’t want to copy across each data member individually, you could put all of the data to copy inside another struct, similar to your don’t copy struct. Put all of the data members you want to copy in the struct and then just copy the struct over the new object.

However, that is not a particularly good design for a class. I would suggest that if you have a class that both has hundreds of data members and requires a copy constructor, you would be better off redesigning the class and breaking it up a bit.

There is also an of an issue having a single non copied data member in a copy constructor. When you copy an object most people would expect that you receive an exact copy of the object, not a partial copy with some stuff missing. This could cause bugs if someone attempts to pass a copy of the object to a function, and had not looked a what the copy constructors were doing.

It might be better, if your class contains data that shouldn’t be copied, to just mark the copy constructors as deleted and make the class non-copyable. If you really need the partial copy behaviour you could add this as a method with a name that more obviously suggests what it is doing.

44stonelions
  • 192
  • 1
  • 10