Daniel
Daniel

Reputation: 2726

Why append a vector to another vector is not allowed here?

I am appending a vector to another vector using the method (c++):

a.insert(a.end(), b.begin(), b.end());

It works, but if b is got from a member function, then it won't work anymore, say

vector<point> const line::returnAVectorOfPoints() const
{
    vector<point> pts;
    // Do something
    return pts;
}

Then this time, when I tried to (something like this)

a.insert(a.end(), returnAVectorOfPoints().begin(), returnAVectorOfPoints().end());

I got a segv. Any ideas what's going wrong here?

Upvotes: 3

Views: 110

Answers (5)

floppy12
floppy12

Reputation: 1053

You could also modify the signature to return a ref.

It avoids an unnecessary copy of your vector

vector<point>& const line::returnAVectorOfPoints() const

Upvotes: 1

SerendipityDoDa
SerendipityDoDa

Reputation: 141

pts is being created on the stack. When the method returns pts goes out of scope.

try something like this:

vector<point> &line::returnAVectorOfPoints(vector<point> &pts) const
{    
    // Do something
    return pts;
}

You would call the method like this

vector<point> a;
returnAVectorOfPoints(a);

This way the vector is external to the method so it can't go out of scope.

I believe the reason it won't work as you wrote is because vector does not have a copy constructor that would allow the compiler to create an intermediary copy of pts before it goes out of scope. Even if vector did have a copy constructor creating that intermediary copy would be an expensive computational process.

Passing a class in by reference avoids unnecessary copies.

Upvotes: 1

juanchopanza
juanchopanza

Reputation: 227370

You are returning a vector by value in line::returnAVectorOfPoints(), so these two iterators are incompatible:

returnAVectorOfPoints().begin(), returnAVectorOfPoints().end()

They point to two different, temporary, objects.

You could store the return value in a temporary variable:

auto v = returnAVectorOfPoints();
a.insert(a.end(), v.begin(), v.end());

As an aside, note you shouldn't return a const value. It inhibits move semantics, and this can be quite costly.

Upvotes: 7

The Forest And The Trees
The Forest And The Trees

Reputation: 1856

It's because you're referencing two different, temporarily-created vectors. Try

auto pts = returnAVectorOfPoints();
a.insert(a.end(), pts.begin(), pts.end());

Upvotes: 2

marcinj
marcinj

Reputation: 49976

This is because your:

line::returnAVectorOfPoints() 

returns each time new instance as temporary value, change to:

vector<point> vec = returnAVectorOfPoints();
a.insert(a.end(), vec.begin(), vec.end());

Upvotes: 2

Related Questions