0

I'm writing the class::operator= for my c++ code, and am trying to clone the class parameter right, in this code snippet.

myClass & myClass::operator =(const myClass &right){
     myClass *abcde = new myClass(right); 
     myClass coolvar(right); //this and the previous line work, but don't update the class

     *this = *abcde; //uses = operator, so infinitely loops

     myClass *this = new myClass(right); // doesn't work [expected unqualified-id]
     myClass this(right); // doesn't work [expected unqualified-id]
}

However, I can't find a way to correctly rewrite the pointers, like using *this = *abcde, or to call the constructors on the class itself like in the bottom two lines. How would I go about this?

joashe
  • 3
  • 3
  • You need to update the fields by hand. These shortcuts trying to leverage the copy constructor won't work. Also you should accept a reference `const myClass &right` to avoid an unnecessary copy constructor call. – John Kugelman Oct 07 '20 at 23:47
  • 2
    Note: By passing in `right` in by value, you were getting close to being able to apply the [Copy and Swap Idiom.](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) – user4581301 Oct 08 '20 at 00:02
  • If I understand your intent, you want to invoke the copy constructor to construct `*this`, which has already been constructed? So that by the time the program ends, there will have been more constructor calls than destructor calls? That is often a recipe for disaster. – JaMiT Oct 08 '20 at 00:14
  • @user4581301 Don't copy and swap by default. It kills your performance quite often. – Passer By Oct 08 '20 at 08:45
  • don't use manual memory management and all your problems just go away. – bolov Oct 08 '20 at 09:40
  • @PasserBy I won't argue that can be overkill, but it's really easy to write, easy to understand, and really hard to mess up. I use it as my starting point and if profiling says I need something faster, I replace it and lost almost nothing because I only invested a few seconds in it. In the meantime, I have code up and running and under test. – user4581301 Oct 08 '20 at 15:04
  • @user4581301 But performant assignment operators are trivially easy to write, so trivial there's a default there for you, which is what you want a lot of the time. Copy and swap is not a time save by any means. – Passer By Oct 08 '20 at 17:10
  • Agreed, but it's not the simple assignments that are the problem. It's the complicated ones people screw up. But that raises the next question: Why are you assigning a complicated class at such a high frequency that speed maters? Probably a design problem that needs to be addressed there. Also remember that the line that separates complicated from trivial is a personal barrier. If you need an assignment operator, and you can't just loop at the class and go, "Duh!" and crank the sucker out, start with copy and swap and save a bunch of debugging.. – user4581301 Oct 08 '20 at 17:34
  • Keep maintenance in mind as well. Your trivial assignment operator may be a complete mind for the person who follows you. – user4581301 Oct 08 '20 at 17:35

1 Answers1

-1

I don't think you really want to construct an object. The proper way to do is to provide an user-defined std::swap and on a newly constructed object use:

std::swap(*this, other);

See: What is the copy-and-swap idiom?

If you want to copy the data in the object you can also try std::memcy.

Adam Hunyadi
  • 1,890
  • 16
  • 32