Does std::vector<Object> reserve method need a copy constructor for Object class?

Question

I'm trying to reserve some spots into an std::vector, and having an error I don't understand:

#include <iostream>
#include <string>
#include <vector>

using namespace std;

class Foo{
public:    
    std::string str;
    int i;

    Foo(){
        this->str = "";
        this->i = 0;
    }

    Foo(Foo &copyFoo){
        this->str = copyFoo.str; //Or get methods if private members
        this->i = copyFoo.i;
    }

    Foo(std::string str, int i){
        this->str = str;
        this->i = i;
    }
};

int main()
{
   std::vector<Foo> fooVector;
   fooVector.reserve(20); // Error

   for(int i=0; i<20; i++){
       fooVector[i] = Foo("Test", i); // Or should I use operator new?
       // Or should I even stick to the push_back method?
   }

   return 0;
}

Of course I could just not reserve, and it would probably work. But now I'm interested in why it is not working now. I added the copy constructor because it looked that could be the problem I was having back then. But after adding the copy constructor, it doesn't work either.

The error says:

In file included from

/usr/local/gcc-4.8.1/include/c++/4.8.1/vector:62:0,

             from main.cpp:3: /usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_construct.h: In

instantiation of 'void std::_Construct(_T1*, _Args&& ...) [with _T1 =

Foo; _Args = {Foo}]':

/usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_uninitialized.h:75:53:

required from 'static _ForwardIterator

std::__uninitialized_copy<TrivialValueTypes>::_uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = std::move_iterator; _ForwardIterator = Foo*; bool

_TrivialValueTypes = false]' /usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_uninitialized.h:117:41:

required from '_ForwardIterator

std::uninitialized_copy(_InputIterator, _InputIterator,

_ForwardIterator) [with _InputIterator = std::move_iterator; _ForwardIterator = Foo*]' /usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_uninitialized.h:258:63:

required from '_ForwardIterator

std::__uninitialized_copy_a(_InputIterator, _InputIterator,

_ForwardIterator, std::allocator<_Tp>&) [with _InputIterator = std::move_iterator; _ForwardIterator = Foo*; _Tp = Foo]'

/usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_vector.h:1142:29:

required from 'std::vector<_Tp, _Alloc>::pointer std::vector<_Tp,

_Alloc>::_M_allocate_and_copy(std::vector<_Tp, _Alloc>::size_type, _ForwardIterator, _ForwardIterator) [with _ForwardIterator = std::move_iterator; _Tp = Foo; _Alloc = std::allocator;

std::vector<_Tp, _Alloc>::pointer = Foo*; std::vector<_Tp,

_Alloc>::size_type = long unsigned int]' /usr/local/gcc-4.8.1/include/c++/4.8.1/bits/vector.tcc:75:70:

required from 'void std::vector<_Tp, _Alloc>::reserve(std::vector<_Tp,

_Alloc>::size_type) [with _Tp = Foo; _Alloc = std::allocator; std::vector<_Tp, _Alloc>::size_type = long unsigned int]'

main.cpp:31:24: required from here

/usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_construct.h:75:7:

error: no matching function for call to 'Foo::Foo(Foo)'

 { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }

   ^ /usr/local/gcc-4.8.1/include/c++/4.8.1/bits/stl_construct.h:75:7:

note: candidates are: main.cpp:22:5: note: Foo::Foo(std::string, int)

 Foo(std::string str, int i){

 ^ main.cpp:22:5: note:   candidate expects 2 arguments, 1 provided main.cpp:17:5: note: Foo::Foo(Foo&)

 Foo(Foo &copyFoo){

 ^ main.cpp:17:5: note:   no known conversion for argument 1 from 'Foo' to 'Foo&' main.cpp:12:5: note: Foo::Foo()

 Foo(){

 ^ main.cpp:12:5: note:   candidate expects 0 arguments, 1 provided

Where is the problem? Do I need to initialize the std:vector objects, or just assign each position to an object instance?

EDIT: I'm using C++11. And If I remove the copy constructor, I receive the following error at the reserve method line:

required from 'void std::_Construct(_T1*, _Args&& ---) [with _TI = Foo; _Args = {Foo&}]

That's why I wrote the copy constructor in the first place.

I don't want to use the resize method because I want the size method to return the actual number of Foo objects contained into the vector, not the amount I reserved.

Answer 1

   std::vector<Foo> fooVector;
   fooVector.reserve(20); // Error

   for(int i=0; i<20; i++){
       fooVector[i] = Foo("Test", i); // Or should I use operator new?
       // Or should I even stick to the push_back method?
   }

The code above is wrong. You are accessing elements beyond size() in the container. The idiomatic way of doing that would be doing push_back/emplace_back on the container to actually create the objects, rather than only memory:

   for(int i=0; i<20; i++){
       fooVector.emplace_back("Test", i);  // Alternatively 'push_back'
   }

Other than that, in C++11 the type used in the container requires either a copy or move constructor.

Answer 2

Change

Foo(Foo &copyFoo) // bah!!! This can't make copies from temporaries

to

Foo(const Foo &copyFoo)  // now that's one good-looking copy constructor

Answer 3

The first problem is that your copy constructor takes its argument by non-const reference, which prevents copying from temporaries.

In this case, there's no need to declare a copy constructor at all: if you don't, then one will be implicitly defined to copy each member, which is what you want.

If the class were sufficiently complicated to need a non-default copy constructor, then it should be

Foo(Foo const &);

Note that you don't necessarily need a copy constructor to store a type in a vector; a move constructor is sufficient if you don't want your class to be copyable.

The second problem is that you're accessing uninitialised elements of the vector. reserve just reserves memory for that many elements without initialising them; you need to insert elements (with resize, insert, push_back, emplace_back or whatever) before you can access them.

Answer 4

As noted by Luchian Grigore in his answer, first you should fix your copy constructor. Or just remove it, because the default one will do just fine here. Secondly, to use the code as you have it now you should use resize instead of reserve:

std::vector<Foo> fooVector;
fooVector.resize(20);

The latter only reserves memory, but container's size remains the same - 0 in your case so it's illegal to access elements. resize on the other hand does what you intended.

But in general you shouldn't even do that (as noted here for example). Just use push_back or emplace_back on an empty vector to fill it with contents. resize will call the default constructor for each element you intend to overwrite anyway.