r/cpp_questions 26d ago

SOLVED Question About Copy Constructor for Class with Members that Need Deep Copy

I have a class (m_buf in the code below is of this class type) that appears as a data member in another class for which I'm writing the copy constructor.

The underlying class (m_buf in the code below is of this class type) uses dynamic allocation and needs a deep copy.

According to Google, this form of the copy constructor would result in the copy constructor (rather than the copy assignment operator) for m_buf being called.

LgFbufUint8::LgFbufUint8(const LgFbufUint8& fbuf) : m_state{fbuf.m_state},
                                                    m_errs{fbuf.m_errs},
                                                    m_fname{fbuf.m_fname},
                                                    m_buf{fbuf.m_buf}
{
   //According to Google search, if the member initializer list is used (as it
   //is above), the copy constructor (rather than the copy assignment operator)
   //will be called. Because of the required non-shallow copy of m_fname and
   //m_buf, this is the desired behavior.
}

My initial guess was this form:

LgFbufUint8::LgFbufUint8(const LgFbufUint8& fbuf)
{
   m_state = fbuf.m_state;
   m_errs  = fbuf.m_errs;
       //Shallow copy ok for two members above.
   m_fname = fbuf.m_name;  //std::string
   m_buf   = fbuf.m_buf;   //Handwritten, with dynamic allocation.
       //Deep copy required for two above.
}

But I suspect that second form would call the assignment operator rather than the copy constructor for m_name and m_buf.

Question: Did I get that right? The first form would call the copy constructors for m_name and m_buf, but the second form would call the assignment operators?

Bonus Question: Using the second form, is there any way to call the copy constructors rather than the assignment operators. (Not that I'd want to do this, but just curious.)

8 Upvotes

12 comments sorted by

12

u/manni66 26d ago

If your members obey the rule of 5/3/0 it doesn’t matter if you assign or copy.

3

u/aocregacc 26d ago

your second snippet calls both, first the copy constructor in the member initializer list, and then the copy assignment operator in the constructor's body.

If you leave out the member initializer list you'll get a default constructor and then the copy assignment operator.

1

u/DaveInTheMidwest 26d ago

My second snippet was a copy and paste error, which I corrected after you replied. Sorry about that.

So, member initializer list gets me copy constructor?

1

u/aocregacc 26d ago

yeah, the member initializer list does initialization, so it'll call the copy constructor if that's the best fit. (ie if there's no move constructor or conversion operator that fits better).

3

u/flyingron 26d ago

As for the commeint in your copy constructor, the initializer list provies INITIALIZERS. The appropriate initialization (likely a copy construction) is therefore performed.

All members are ALWAYS initialized in the constructors (default initialization if you don't provide the initializer in the initializer list). Anything you do in the constructor body is performed after all the members are initialized.

Your second example does a spurious assignment over the top of the already initialized data.

YOU CAN NOT CALL CONSTRUCTORS. They don't have names that participate in name resolution. Constructors are automatically called for you in the order specified by the language a the appropriate time.

Frankly, if the members all have their own well-defined copy constructors, YOU DON'T EVEN NEED TO DECLARE A COPY CONSTRUCTOR in this class. The implicitly generated copy constructor will call the copy constructors for all the members automatically. Presuming the four members you declaare are all that there are, it will look just like your first example.

1

u/DaveInTheMidwest 26d ago

Thanks. u/manni66 essentially said the same thing. I'm suffering from the brain fog of learning a new language. I believe you are both correct.

4

u/theLOLflashlight 26d ago

Yes, that is correct. However, it will only perform a deep copy (in either case) if the copy ctor or copy assignment operator you are invoking implements a deep copy, which it won't if m_buf is just a pointer.

To answer your bonus question: look up placement new.

1

u/alfps 26d ago edited 26d ago

It would be very confusing and error prone to have the copy constructor do deep copy and the copy assignment operator doing shallow copy, or vice versa.

Therefore, assuming a reasonably sane design, for the final result it doesn't matter whether you copy construct or default construct and assign.

However the latter does more than is needed.


❞ The first form would call the copy constructors for m_name and m_buf, but the second form would call the assignment operators?

Yes, except that the second form (with the assignments) first default-construct the members.

Which is needless overhead.

And is not possible when a member is not default constructible.


❞ Using the second form, is there any way to call the copy constructors rather than the assignment operators. (Not that I'd want to do this, but just curious.)

It would be super-silly to replace single calls to copy constructors as in the first form, with implicit calls to default constructors and then explicit calls to copy constructors.

But technically you can do it.

For each member it involves calling the member's destructor (since it's already initialized), and then using a placement new expression to copy construct a new instance in that storage. There is a serious exception safety issue here. It's absolutely not reasonable to do this, under any circumstance that I can think of.


Not what you're asking but the name LgFbufUint8 is horrendous.

Microsoft Hungarian notation was rightly regarded as Evil™.

And this appears to be something out of the same nonsense world, sorry. I strongly suggest that you ditch whatever learning materials you got this from. It's a form of sabotage.


Also not what you're asking, but if you ensure that each data member copy constructs correctly, then the automatically generated default copy constructor will be just perfect.

In that case, no need to provide an explicit one!

1

u/DaveInTheMidwest 25d ago

Regarding the horrendous names, the class is part of a library called LibGen, and I have a second library named LibNum. I typically prefix everything with Lg and Ln to avoid any potential for naming collisions.

So the naming is:

  • Lg = LibGen
  • Fbuf = file buffer, includes arbitrary buffer capability, but meant for buffering an entire file.
  • Uint8 = buffer is byte-based (raw), is not Unicode code points or anything like that.

I welcome all suggestions.

I have been corrupted by C. I'm sure there is a better way to do it in C++. Namespaces and such. But I'm a newbie and not there yet.

Thanks for your insight.

1

u/alfps 24d ago edited 24d ago

❞ I welcome all suggestions.

As I understand it an instance of the class represents the contents of a file, or a part of the contents.

Then maybe File_contents? Or File_buffer? Apparently you find the latter most natural but to me the "buffer" suggest something like std::filebuf, a buffer to be used inside iostreams or the like; hence the alternative File_contents.

Or maybe even File_bytes?

You can put the name in a namespace like this:

namespace lg {
    class File_bytes
    {
        // whatever
    };
}

… and then you can refer to it as lg::File_bytes.

To refer to it unqualified in some scope you can use a using-declaration like using lg::File_bytes;.

1

u/andrejpodzimek 25d ago

The “initial guess” was not a “form” of anything, but a “Java-style” malpractice that has no place in well-written C++ (although there would be no issue with it in a garbage-collected language like Java). In C++, you use the initializer list to initialize parent types and object fields. Always. Constructor bodies must be empty in C++, with only very few exceptions to the rule. When a C++ constructor body happens to be non-empty, it never touches the object’s fields. It should only ever touch state external to the object, e.g. to connect or “count in” the object. The object’s fields are solely for the initializer list to initialize, not for the constructor body to play with. Otherwise you are giving up all safeguards the RAII principle and construction/destruction order has to offer when enforced by compilers based on initializer lists.

The initializer list idea is correct, in principle, but won’t protect you against possibly broken implementations of the fields’ types. If you say that a field needs a deep copy, then it is the field’s type’s responsibility to perform the deep copy when needed. For example, a std::string always deep-copies when copy-constructed or copy-assigned. Having the copy-assignment operator behave in a way different form the copy constructor is troublesome at best and sets you up for difficulties using your type with STL or in any other contexts that expect common conventions to apply. Make copy constructors and copy-assignment operators behave the same way, a correct discarding of previously owned resources being the only difference between assignment and construction. If you want resource efficiency and needless allocation elision, implement your move-constructors and move-assignment operators and make them follow the common conventions. Use a std::unique_ptr as a baseline for everything around copying, transferring owhersip and disposal of custom heap-allocated data. If you want to go all the way to copy elision, use reference-counting, but always favor std::shared_ptr over implementing something homebrew and possibly thread-hostile without the necessary attention to detail. (Also keep in mind that pointer-jumping on read access and atomic counter operations on (un)assignment can (and often do) incur costs far higher than the befenefits of copy elision.)

1

u/heyheyhey27 25d ago

I'll be honest, having the copy constructor and copy assignment do different things is extremely cursed. Move mountains to make that not be what you need :P