Inserting a std::vector as returned from a function

Question

I find myself frequently doing something like this to concatenate several vectors that are returned from functions (possibly class functions):

#include <vector>
#include <iostream>
using namespace std;

vector<int> v1;

const vector<int>& F1() {
    cout << "F1 was called" << endl;
    /*Populate v1, which may be an expensive operation*/
    return v1;
}

int main() {
    vector<int> Concat;
    Concat.insert(Concat.end(), F1().begin(), F1().end());
    /*Do something with Concat*/
    return 0;
}

As I expected, F1() is called twice, which may be undesirable if it is an expensive function call. An alternative is to copy the return value of F1() into a temporary vector which would only require one function call, but would incur a copy operation which might be undesirable if the vector is large. The only other alternative I can think of is to create a pointer to a temporary vector and assign the return value of F1() to it like this:

int main() {
    vector<int> Concat;
    const vector<int>* temp = &F1();
    Concat.insert(Concat.end(), temp->begin(), temp->end());
    /*Do something with Concat*/
    return 0;
}

Is this really the best solution? The use of a temporary variable seems cumbersome, especially if I need to concatenate several vectors. I also feel like there should be a way to do this using references instead of pointers. Any suggestions?

Answer 1

The best solution is not to use vector directly in the first place but OutputIterators and std::back_inserter.

template <typename OutputIterator>
OutputIterator F1( OutputIterator out )
{
    cout << "F1 was called" << endl;
    /* Insert stuff via *out++ = ...; */
    *out++ = 7;
    return out;
}

int main()
{
    std::vector<int> Concat;
    // perhaps reserve some moderate amount of storage to avoid reallocation

    F1( std::back_inserter(Concat) );
    F1( std::back_inserter(Concat) );
}

Demo. This way maximum efficiency and flexibility are achieved.

Answer 2

Is this really the best solution?

No. std::vector supports move semantics, so you should do this instead:

vector<int> F1() // return by value
{
    std::vector<int> v1; // locally declared here
    cout << "F1 was called" << endl;
    /*Populate v1, which may be an expensive operation*/
    return v1;
}

int main() {
    vector<int> Concat;
    auto v2 = F1(); // create local variable and assign result of F1
    Concat.insert(Concat.end(), v2.begin(), v2.end());
    /*Do something with Concat*/
    return 0;
}

This code:

• doesn't depend on global variables defined elsewhere
• is efficient (F1() gets called once)
• is simple and straightforward

Edit: On second thought, go with @Columbo's approach. It is more idiomatic, more flexible, and more efficient.

Answer 3

vector<int> F1() {
    cout << "F1 was called" << endl;
    vector<int> v1;
    /*Populate v1, which may be an expensive operation*/
    return v1;
}

int main() {
    vector<int> Concat;
    vector<int> v1 = F1();
    Concat.insert(Concat.end(), v1.begin(), v2.end());
    /*Do something with Concat*/
    return 0;
}

this will be even faster than your original code. Returning a vector shouldn't copy it in C++11, you're guaranteed that if movable, it will be moved. Moreover in this particular case NRVO (Namer return value optimization) will kick in.

However I would do it this way:

void F1(vector<int>& v1) {
    cout << "F1 was called" << endl;
    /*Populate v1, which may be an expensive operation*/
    return;
}

and you just concatenate directly into v1 in the method.