C++ appending to vector of strings efficiently (and idiomatically)

Question

If I want to fill a vector of strings with lines from a file in C++, is it a good idea to use push_back with std::move?

{
    std::ifstream file("E:\\Temp\\test.txt");
    std::vector<std::string> strings;
    
    // read
    while (!file.eof())
    {
        std::string s;
        std::getline(file, s);
        strings.push_back(std::move(s));
    }

    // dump to cout
    for (const auto &s : strings)
        std::cout << s << std::endl;
}

Or is there some other variant where I would simply append a new string instance to the vector and get its reference?

E.g. I can do

std::vector<std::string> strings;
strings.push_back("");
string &s = strings.back();

But I feel like there must be a better way, e.g.

// this doesn't exist
std::vector<std::string> strings;
string & s = strings.create_and_push_back();

// s is now a reference to the last item in the vector, 
// no copying needed

As a side note, your usage of `while (!file.eof())` is [wrong](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons). — MikeCAT, Feb 08 '21 at 12:37
`strings.push_back(std::move(s));` is a quite cheap operation. Your other approach - first to `push_back()` and then to use `back()` for input - is cheap as well. However, there is something you didn't consider: You should handle the case where input failed. In your first approach, you simply could bail out. (This would make the `!file.eof()` obsolete btw.) In the second approach, don't forget to pop the (now obsolete) empty string you already pushed for storage of input (before you bail out). — Scheff's Cat, Feb 08 '21 at 12:42
Doesn't `emplace_back()` does just what you want (assuming C++17)? — Igor G, Feb 08 '21 at 12:48

score 3 · Accepted Answer · answered Feb 08 '21 at 12:44

Except for the eof misuse, this is the pretty much the idiomatic way to do it yes. Below is the correct code:

std::string s;
while(std::getline(file, s))
{
    strings.push_back(std::move(s));
    s.clear();
}

Note the explicit s.clear() call: the only guarantee you have for a moved-from object std::string is that you can call member functions with no prerequisites, so clearing a string should reset it to a "fresh" state, as the move is not guaranteed to do anything to the object, and you can't rely on getline not doing anything weird.

There are some other ways to spell this out (you can probably achieve something similar with istream_iterator and a proper whitespace setting), but I do think this is the clearest.

one major difference in your answer that is worth mentioning is that the string object is outside the loop and gets reused instead of being created and destroyed at each loop iteration. — bolov, Feb 08 '21 at 12:59

C++ appending to vector of strings efficiently (and idiomatically)

1 Answers1