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);
...

Solution 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.

Solution 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"};
}

Solution 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.

Solution 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.

View more solutions

Comments

[Andres Bucci]

This gives me an error if I try to do it like this, "No matching constructor for initialization of 'std::vector<std::string>'". Initially i thought that I should do it like you say, but it doesn't compile.

[juanchopanza]

@AndresBucci you would need std::vector<std::string> (types , types + sizeof(types)/sizeof(std::string));

[endorphin]

@AndresBucci - see edits, juanchopanza's comment beat my edit by a bit. Basically, vector doesn't have a (Type*, nelements) constructor, it's got a (iterator, iterator) constructor.

[Andres Bucci]

I wanted to select your answer as the accepted one, but since the one I selected contains an explanation I think it's gonna be better for the future readers.

[James Kanze]

What's wrong with using std::being( types ), std::end( types ) as the argument to the constructor of the vector? (If you don't have C++11, of course, you use the corresponding functions in your toolkit.)

[James Kanze]

RVO applies equally to all of the code. Even with "older" compilers (many of which actually did it better than some of the more recent compilers).