Is there a more efficient way to set std :: vector from a stream?

Question

I am currently setting a std::vector<char> from std::ostringstream as follows:

 void foo(std::vector<char> &data, std::stringstream &stream) { data = std::vector<char>(stream.str().begin(), stream.str().end()); } 

I am wondering if there is a more efficient way to do this with STL in C ++, or is the method that I give here considered appropriate? Would I be better off using std::stringstream instead?

Answer 1

Your method invokes undefined behavior. stream.str() returns a string by value, such as a temporary string. You accept the begin iterator of one temporary end iterator of the other, creating an invalid range.

One way to convert a stream to a container is to use a common iterator interface:

 #include <iostream> #include <sstream> #include <vector> #include <algorithm> #include <iterator> int main(){ std::stringstream src("...."); std::vector<char> dest; // for a bit of efficiency std::streampos beg = src.tellg(); src.seekg(0, std::ios_base::end); std::streampos end = src.tellg(); src.seekg(0, std::ios_base::beg); dest.reserve(end - beg); dest.assign(std::istreambuf_iterator<char>(src), std::istreambuf_iterator<char>()); std::copy(dest.begin(), dest.end(), std::ostream_iterator<char>(std::cout)); } 

Live example on Ideone.

Another method would be to cache the returned std::string object:

 std::string const& s = stream.str(); data.reserve(s.size()); data.assign(s.begin(), s.end()); 

Answer 2

As stated in the comments, your code is incorrect due to two str() calls. To increase efficiency, you can avoid creating a temporary vector , for example:

 void foo(std::vector<char> &data, std::stringstream &stream) { const std::string& str = stream.str(); data.assign( str.begin(), str.end() ); } 

You can also avoid the std::string using std::istreambuf_iterator s:

 void foo(std::vector<char> &data, std::stringstream &stream) { data.assign( std::istreambuf_iterator<char>( stream ), std::istreambuf_iterator<char>() ); } 

but provided that they are input iterators, vector does not have the ability to find out how much data will be assigned, and may deteriorate a little, since reserve cannot have enough space to avoid redistribution.

Answer 3

if there is a more efficient way

You can call reserve on data and use the insert range member directly on data instead of using copy assignment. The thing about vector that you need to remember is that each node can potentially increase its size (and move all the elements). Thus, you better allocate memory at a time (if you know how many objects you will store - what you know here) and use this fact.

Answer 4

Copy from stream iterator to back paste iterator:

 std::istream src; std::vector<char> dst; std::copy(std::istream_iterator<char>(src), std::istream_iterator<char>(), std::back_inserter(dst)); 

istream_iterator uses a formatted transform (i.e. skips spaces), so this may not be what you want. I'm not sure what your goal is.