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?
The best solution is not to use vector
directly in the first place but OutputIterator
s 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.
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.
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.