Assignment and std::copy of a vector

Question

I have large complex class and I would like to do this

class A
{
///a lot of things
vector<something> vs;
};

and copy constructor

A::A(const A& a)
{
   vs=a.vs;
}

against this

A::A(const A& a)
{
   copy(a.vs.begin(),a.vs.end(),back_inserter(vs));
}

which one more better ?

@CIsForCookies, no, because this uses `back_inserter` which makes it not wrong ... just not optimal. — Jonathan Wakely, Sep 15 '15 at 18:49
@MegumiBear, if the `copy()` version was better don't you think the library implementor would already have changed the assignment operator to use it?! So it can't reasonably be faster or more efficient, so it could only be "better" if you think long, complex expressions are better than short, succinct ones that do the right thing. But as the answers below show, there is a third option that's even better. — Jonathan Wakely, Sep 15 '15 at 18:49
It is better to write your class so that the default generated copy constructor is correct. If you can't, then wrap the complex parts in sub classes which handle their copying correctly, until the final class will work without a custom copy constructor. — Neil Kirk, Sep 15 '15 at 20:58

NathanOliver · Accepted Answer · 2015-09-15T18:51:55.740

5

If you are writing your own copy constructor you should use member initialization and directly construct the vector from the source.

A::A(const A& a) : vs(a.vs) {}

Now if all of your class members are copy constructable then you do not need to have a copy constructor as the one provided by the compiler will be sufficient.

edited Sep 15 '15 at 18:51

answered Sep 15 '15 at 18:05

NathanOliver

171,901
28
288
402

1

big if. even with a `std::vector` member function, the compiler will generate the correct copy constructor for you. – TemplateRex Sep 15 '15 at 18:10
@TemplateRex What is a big if? I am just saying that "if" he is going to make his own copy constructor instead of relying on the default. – NathanOliver Sep 15 '15 at 18:12
1

it's a dangerous remark in the eyes of a novice, before you know it, they *are* writing their own copy constructors. – TemplateRex Sep 15 '15 at 18:13

TemplateRex · Answer 2 · 2015-09-15T20:51:52.603

4

The best way is to rely on neither and let the compiler do its work. It will generate the optimal copy constructor for you, unless you have special data members (yet to be seen). Then, and only then, would you have to worry about writing your own copy constructor (and you can use the member initialization as pointed out by @NathanOliver).

I know it's not your actual question, but closely related to it: if you want to use a regular constructor taking a vector argument, the best way is to use member initialization which will call the copy constructor of vector (and your Standard Library will have written optimal code for that).

class A
{
public:
    A(vector<something> const& v) : vs(v) {}

    // let the compiler generate the copy constructor, UNLESS you have special data members members        
    A(A const& other): vs(other.vs) { /* e.g. deep copy or other special stuff */ }
private:    
    //a lot of things
    vector<something> vs;
};

edited Sep 15 '15 at 20:51

answered Sep 15 '15 at 18:15

TemplateRex

69,038
19
164
304

I think all you need is the first paragraph. OP is not asking about a constructor taking a vector argument so I think the second part is just a distraction (in my experience you rarely want to copy a large vector into a class, if you do want to take ownership of a vector it is better to move it in). – Chris Drew Sep 15 '15 at 18:39
@ChrisDrew I know OP is not asking about regular constructor, but the point is that this would be only point where I would consider writing something myself rather than relying on the compiler. Updated, thanks. Moving the vector would be depending on use case. – TemplateRex Sep 15 '15 at 18:41
Since in the OP, "a lot of things" is stated, then if any of these other things are pointers, do not just let the compiler generate this for you. A compiler's default copy constructor will only do a shallow copy and if you have pointers, you'll need to define your own copy constructor to do a deep copy. – Kirkova Sep 15 '15 at 20:47
@Kirkova thanks, updated, but this should be the exception, not the regular thing to do with most classes. – TemplateRex Sep 15 '15 at 20:53
1

@TemplateRex Yes, I completely agree with you, since it seems like they might be new, I just wanted to clarify. – Kirkova Sep 15 '15 at 20:55
@Kirkova yes, thanks, for newcomers in C++, a little knowledge is a dangerous thing :) – TemplateRex Sep 15 '15 at 20:56

Assignment and std::copy of a vector

2 Answers2