Initializing std::vector of std::string

Question

While working in a project with some legacy code i found this function:

std::vector<std::string> Object::getTypes(){
    static std::string types [] = {"type1","type2", "type3"};
    return std::vector<std::string> (types , types +2);
}

I would probably have written this as:

std::vector<std::string> Object::getTypes(){
    std::vector<std::string> types;
    types.push_back("type1");
    types.push_back("type2");
    types.push_back("type3");
    return types;
}

Is this merely a style choice or is there something I'm missing? Any help would be greatly appreciated. Sorry if this is too basic.

Update: Actually found different classes that override the same method do it one way or the other, so It's even more ambiguous. I would make them all the same but would prefer the better approach, if there is one.

---

Edit

Please note that the above legacy code is incorrect because it initializes the vector with only the first two elements of the array. However, this error has been discussed in the comments and thus should be preserved.

The correct initialization should have read as follows:

...
    return std::vector<std::string> (types, types + 3);
...

Answer 1

The array types in the first example is declared static. This means it only exists once in memory. So there are three options for what to return and they live in static memory. Then, when you create the vector to return, you are able to allocate it's memory in one shot by passing the beginning and ending of the array as iterators.

By doing it this way, you don't have successive calls to push_back which means the vector won't have to reallocate its internal block of memory.

Also, when the vector is constructed as part of the return call, older compilers will have an easier time of doing return value optimization.

Answer 2

If you have a C++11 capable compiler and library, returning an initializer list should be enough:

std::vector<std::string> Object::getTypes(){
    return {"type1","type2", "type3"};
}

Answer 3

The code you found is more efficient (because types[] is only allocated once and push_back can/will cause re-allocations). The difference though is marginal, and unless you call getTypes in a (relatively big) loop it shouldn't matter at all (and probably it won't matter much even when you do call it in a big loop).

As such, unless it creates a concrete performance problem, it's a style choice.

Answer 4

Basically it's a style choice. I'd probably do something more like

std::vector<std::string> Object::getTypes(){
    static std::string types [] = {"type1","type2", "type3"};
    return std::vector<std::string> (types, 
                   types + (sizeof(types)/sizeof(std::string)) );
}

which lets you change the number of things in types, without having to remember to update the count in the next line.

Answer 5

One reason I like to use this style of initialization with iterators (and C++11's uniform initialization and initializer lists) is that it helps separating data from code.

Repeating push_back a lot of times feels bad because I desperately need to refactor that. Also, when you really just need to initialize the container with data, then you want to see a list of the data, and not really code that generates data. The method you found in the original version matches that principle better.