CODEWITHSUNDEEP
c++constructor
SR Bhaskar
SR Bhaskar

Reputation: 908

C++ copy constructor double call on member initialization

Consider the below code, where a composing class with another class as its member is being instantiated:

class CopyAble {
private:
    int mem1;
public:
    CopyAble(int n1) : mem1(n1) {
        cout << "Inside the CopyAble constructor" << endl;
    }
    CopyAble(const CopyAble& obj) {
        cout << "Inside the CopyAble copy constructor" << endl;
        this->mem1 = obj.mem1;
        return *this;     
    }

    CopyAble& operator=(const CopyAble& obj) {
        cout << "Inside the  CopyAble assignment constructor" << endl;
        this->mem1 = obj.mem1;
    }
    ~CopyAble() {};
};

class CopyAbleComposer {
    private:
        CopyAble memObj;
    public:
        CopyAbleComposer(CopyAble m1) : memObj(m1) {
            cout << "Composing the composer" << endl;
        }
        ~CopyAbleComposer() {}
};

int main()
{
    CopyAble ca(10);
    CopyAbleComposer cac(ca);
    return 0;
}

When I run this, I get the output:

Inside the CopyAble constructor  
Inside the CopyAble copy constructor  
Inside the CopyAble copy constructor  
Composing the composer

Which means that the CopyAble copy constructor is being run twice - once when the CopyAble object is passed into the CopyAbleComposer constructor, and again when the initializer memObj(m1) runs.

Is this an idiomatic use of the copy constructor? It seems very inefficient that the copy constructor runs twice when we try to initialize a member object with a passed-in object of the same type, and it seems like a trap a lot of C++ programmers can easily fall into without realizing it.

EDIT: I don't think this is a duplicate of the question regarding passing a reference into the copy constructor. Here, we are being forced to pass a reference into a regular constructor to avoid duplicate object creation, my question was that is this generally known that class constructors in C++ should have objects passed in by reference to avoid this kind of duplicate copy?

Upvotes: 2

Views: 748

Answers (3)

John Ko
John Ko

Reputation: 19

Several issues:

  1. return *this; should not be in the copy constructor. It belongs in the assignement operator.
  2. CopyAbleComposer(CopyAble m1) is where a needless copy of ca is made. It should instead be CopyAbleComposer(const CopyAble& m1).

Upvotes: 0

anatolyg
anatolyg

Reputation: 28241

Pass-by-value and the associated copying is a pretty widely known property of C++. Actually, in the past C++ was criticized for this gratuitious copying, which happened silently, was hard to avoid and could lead to decreased performance. This is humorously mentioned e.g. here:

You accidentally create a dozen instances of yourself and shoot them all in the foot. Providing emergency medical assistance is impossible since you can't tell which are bitwise copies and which are just pointing at others and saying, "That's me, over there."

C++98

When any function/method is declared to receive an argument by value, this sort of copying happens. It doesn't matter if it's a constructor, a "stand-alone" function or a method. To avoid this, use a const reference:

CopyAbleComposer(const CopyAble& m1) : memObj(m1)
{
    ...
}

Note: even if you rearrange your code as below, one copy always remains. This has been a major deficiency in C++ for a long time.

CopyAbleComposer cac(CopyAble(10)); // initializing mem1 by a temporary object

C++11

C++11 introduced move semantics, which replaces the additional copy by a "move" operation, which is supposed to be more efficient than copy: in the common case where an object allocates memory dynamically, "move" only reassigns some pointers, while "copy" allocates and deallocates memory.

To benefit from optimization offered by move semantics, you should undo the "optimization" you maybe did for C++98, and pass arguments by value. In addition, when initializing the mem1 member, you should invoke the move constructor:

    CopyAbleComposer(CopyAble m1) : memObj(std::move(m1)) {
        cout << "Composing the composer" << endl;
    }

Finally, you should implement the move constructor:

CopyAble(CopyAble&& obj) {
    cout << "Inside the CopyAble move constructor" << endl;
    this->mem1 = obj.mem1;
}

Then you should see that the "copy" message doesn't appear, and is replaced by the "move" message.

See this question for more details.

Note: In all these examples, the CopyAble objects are assumed to be much more complex, with copy and move constructors doing non-trivial work (typically, resource management). In modern C++, resource management is considered a separate concern, in the context of separation of concerns. That is, any class that needs a non-default copy or move constructor, should be as small as possible. This is also called the Rule of Zero.

Upvotes: 2

user7860670
user7860670

Reputation: 37485

You should accept CopyAble by reference at CopyAbleComposer(CopyAble m1), otherwise a copy constructor will be called to construct an argument. You should also mark it as explicit to avoid accidental invocations:

explicit CopyAbleComposer(const CopyAble & m1)

Upvotes: 2

Related Questions