copy constructor - am I setting my std::strings correctly

Question

My question is a basic one. Since std::strings are arrays am I handling them correctly in my copy constructor?

class json{
    private:
        std::string _objectContents;
        std::string _regComments;

        bool _isJson;
        int numElements;

    public:
        json(const json&);

        ...
};

json::json(const json& source){
    _objectContents = source._objectContents;
    _regComments = source._regComments;
    _isJson = source._isJson;
    _numElements = source.numElements;
}

*edited remove pointer and changed to array in question.

1) Strings are not pointers. 2) Copy constructors are generated by the compiler, you almost never have to write them yourself. — , Jan 20 '15 at 17:37
The default implementation of the copy constructor should be fine in this case, you don't need to write your own. — Borgleader, Jan 20 '15 at 17:37
+1 to @Borgleader, but if you still want to write copy constructor, initialize members using an initializer list, not in the body. — ichramm, Jan 20 '15 at 17:38
To be very clear since it's not in the top answer, a `std::string` is **not** a pointer. And it does not matter if any pointers happen to be used in their implementation. — Drew Dormann, Jan 20 '15 at 17:49
wow i still get downvoted even if i paste my code. what is the secret to not getting trolled on here? — user249806, Jan 20 '15 at 17:55
I didn't downvote but it was likely to do with no prior research. Also, it has nothing at all to do with "trolling" — Lightness Races in Orbit, Jan 20 '15 at 17:58

score 3 · Answer 1 · answered Jan 20 '15 at 17:39

am i setting my std::strings correct

You are setting them correct but it can be better.

You don't need an explicitly defined copy constructor. The one generated by the compiler will do the right thing for the member variables you have.

If you must explicitly define the copy constructor, initialize the members using initialization list. Change the implementation to:

json::json(const json& source) : 
   _objectContents(source._objectContents),
   _regComments(source._regComments),
   _isJson(source._isJson),
   _numElements(source.numElements)
{
}

thanks! i keep getting an error when i was using the string constructor inside the method. Your answer makes perfect sense. I see now that I do not need to explicitly do this for each string, also. — user249806, Jan 20 '15 at 17:50

Aabesh Karmacharya · Answer 2 · 2015-01-20T18:00:01.830

The string assignment operator implicitly copies(or moves) the underlying character array from the source string to the destination string(a deep copy) rather than copying just their pointers(a shallow copy).
so Yes you the code is correct.
Since all of your member variables can be copied using an assignment operator you do not need to explicitly declare a copy constructor.
Though you would have needed a copy constructor had you used a C style string. In which case the default copy constructor does a shallow copy while you may be intending to do a deep copy.

copy constructor - am I setting my std::strings correctly

2 Answers2