C : copying a struct containing a std::vector efficiently-CodePudding

struct MZEntry
{
    uint32_t machineID;
    bool mode;
    uint32_t area;
    uint32_t occupancy;

    
    using ZList = std::vector<uint32_t>;
    
    ZList authorisationZ;
    ZList blockExceptionZ;
    
    void clear()
    {
        machineID = 0;
        mode = false;
        area = 0;
        occupancy = 0;
        authorisationZ.clear();
        blockExceptionZ.clear();
    }

    MZEntry(){
        clear();
    }

    MZEntry(const MZEntry& mzEntry)
    {
        machineID = mzEntry.machineID;
        mode = mzEntry.mode;
        area = mzEntry.area;
        occupancy = mzEntry.occupancy;
        authorisationZ = mzEntry.authorisationZ;
        blockExceptionZ = mzEntry.blockExceptionZ;
    }
};

I have the above structure in my code. Is there any benefit in me explictly declaring the copy-constructor?

Let's say I have two variables

MZEntry entry_1;
MZEntry entry_2;

if I do entry_1 = entry_2, will all the fields in the entry_2 get copied over to entry_1 including the vectors in it?

Or is an explict copy-constructor required. Also is the above explict copy-constructor more efficient?

CodePudding user response：

Declaring a copy constructor is a bad idea. Follow the rule-of-zero whenever you can and don't declare any copy/move constructor or assignment operator or destructor if you don't need custom destructor behavior because the class must manage some resource (and if it has to, always encapsulate that in a class specifically limited to that purpose, following the rule-of-five (or rule-of-three)). See also the C core guidelines.

If you declare the copy constructor manually you inhibit the implicit definition of the move operations which would be used instead of a copy operations whenever possible to perform a move efficient move (rather than copy) of the members.

For example

MZEntry entry_1;
MZEntry entry_2;
/*...*/
entry_1 = std::move(entry_2);

With the (implicit) move assignment operator, this allows entry_1 to just take over the memory which the vector members of entry_2 allocated. entry_2 will afterwards be in an unspecified state, but that is often fine. With the manual declaration of your copy constructor there won't be an implicit move assignment operator and the above would perform a full copy of all vector elements, the same as entry_1 = entry_2 would (with or without your manually defined copy constructor).

Also, as mentioned in the question comments, your implementation of the copy constructor is actually worse performance-wise than the compiler-generated one, although it produces exactly the same behavior in the end.

Similarly manual definition of the default constructor can be avoided by using in-class default member initializers instead and letting the compiler generate the default constructor from that.

Calling .clear() on the vector members is redundant, since their default constructor will put them into an empty state anyway.

Following the core guidelines you would have:

struct MZEntry
{
    uint32_t machineID = 0;
    bool mode = false;
    uint32_t area = 0;
    uint32_t occupancy = 0;

    using ZList = std::vector<uint32_t>;
    
    ZList authorisationZ;
    ZList blockExceptionZ;
};

or alternatively (same result, matter of style):

struct MZEntry
{
    uint32_t machineID{};
    bool mode{};
    uint32_t area{};
    uint32_t occupancy{};

    using ZList = std::vector<uint32_t>;
    
    ZList authorisationZ{};
    ZList blockExceptionZ{};
};

which lets the compiler generate all of the special member functions optimally. And if you need the clear method for some other purpose, it can then be implemented simply as

void clear() {
    *this = MZEntry();
}

or (again matter of style, same result):

void clear() {
    *this = {};
}

(Although your more explicit implementation of the clear member function as such might be more performant overall, since it won't cause memory of the vectors to be released, so that they may then reuse it later without new allocations.)