6

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?

Carlton
  • 4,217
  • 2
  • 24
  • 40
  • " I also feel like there should be a way to do this using references instead of pointers." there is. Just use reference. – Sopel Nov 20 '14 at 14:24
  • If I use references by changing `const vector* temp = &F1();` to `const vector& temp = F1();`, will this incur a copy operation on the vector? – Carlton Nov 20 '14 at 14:34
  • Only one. When you insert it the vector must be copied to the end of the Concat (and probably a realocation will occur). – Sopel Nov 20 '14 at 16:26

3 Answers3

5

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.

Columbo
  • 60,038
  • 8
  • 155
  • 203
  • Can you add how this works? – ZoomIn Nov 07 '21 at 15:44
  • This addresses the original question, but is still not a generally satisfying solution. In some cases one might just want to append the returned vector of an existing function that cannot be changed. – euphrat Nov 28 '21 at 01:45
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.

utnapistim
  • 26,809
  • 3
  • 46
  • 82
  • Thanks for your answer. I agree, this is very straightforward and easy to read. In some cases I'm returning a vector which is a private class member, so I don't think I can use move semantics in that case unfortunately. – Carlton Nov 20 '14 at 14:48
  • No, in such a case (private member) it usually doesn't make sense to return by value. – utnapistim Nov 20 '14 at 14:52
1
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.

dau_sama
  • 4,247
  • 2
  • 23
  • 30
  • Thanks. The general consensus seems to be to pass the vector to the function instead of inserting the return value into the vector. – Carlton Nov 20 '14 at 14:52