Search code examples
c++stringvectorfilestream

c++ String from file to vector - more elegant way


I write a code in which I want to pass several strings from text file to string vector. Currently I do this that way:

using namespace std;
int main()
{
string list_name="LIST";
ifstream REF;
REF.open(list_name.c_str());
vector<string> titles;
for(auto i=0;;i++)
{
    REF>>list_name;
    if(list_name=="-1"){break;}
    titles.push_back(list_name);
}
REF.close();
cout<<titles.size();
for(unsigned int i=0; i<titles.size(); i++)
{
    cout<<endl<<titles[i];
}

It works fine, I get the output as expected. My concern is is there more elegant way to pass string from text file to vector directly, avoiding this fragment, when passing string from filestream to string object and assigning it to the vector with push_back as separate step:

REF>>list_name;
if(list_name=="-1"){break;}
titles.push_back(list_name);

Solution

  • The other answers are maybe too complicated or too complex.

    Let me first do a small review of your code. Please see my comments within the code:

    #include <iostream>
    #include <fstream>
    #include <string>
    #include <vector>
    
    using namespace std;  // You should not open the full std namespace. Better to use full qualifiacation
    int main()
    {
        string list_name = "LIST";
        ifstream REF;                  // Here you coud directly use the construct ofr the istream, which will open the file for you
        REF.open(list_name.c_str());   // No need to use c_str
        vector<string> titles;         // All variables should be initialized. Use {}
        for (auto i = 0;; i++)         // Endless loop. You could also write for(;;), but bad design
        {
            REF >> list_name;
            if (list_name == "-1") { break; }   // Break out of the endless loop. Bad design. Curly braces not needed
            titles.push_back(list_name);
        }
        REF.close();                   // No nbeed to close the file. With RAII, the destructor of the istream will close the file for you
        cout << titles.size();
        for (unsigned int i = 0; i < titles.size(); i++)  // Better to use a range based for loop 
        {
            cout << endl << titles[i];   // end not recommended. For cout`'\n' is beter, because it does not call flush unneccesarily.
        }
    }
    

    You see many points for improvement.

    Let me explain some of the more important topics to you.

    • You should use the std::ifstreams constructor to directly open the file.
    • Always check the result of such an operation. The bool and ! operator for the std::ifstream are overwritten. So a simple test can be done
    • Not need to close the file. The Destructor of the std::ifstream will do that for you.
    • There is a standard approach on how to read a file. Please see below.

    If you want to read file until EOF (end of file) or any other condition, you can simply use a while loop and call the extraction operator >>

    For example:

    while (REF >> list_name) {
        titles.push_back(list_name);
    }
    

    Why does this work? The extraction operator will always return a reference to the stream with what it was called. So, you can imagine that after reading the string, the while would contain while (REF), because REF was returned by (REF >> list_name. And, as mentioned already, the bool operator of the stream is overwritten and returns the state of the stream. If there would be any error or EOF, then if (REF) would be false.

    So and now the additional condition: A comparison with "-1" can be easily added to the while statement.

    while ((REF >> list_name) and (list_name != "-1")) {
        titles.push_back(list_name);
    }
    

    This is a safe operatrion, because of boolean short-cut evaluation. If the first condition is already false, the second will not be evaluated.

    With all the knwo-how above, the code could be refactored to:

    #include <iostream>
    #include <fstream>
    #include <string>
    #include <vector>
    
    int main() {
    
        // Here our source data is stored
        const std::string fileName{ "list.txt" };
    
        // Open the file and check, if it could be opened
        std::ifstream fileStream{ fileName };
        if (fileStream) {
    
            // Here we will store all titles that we read from the file
            std::vector<std::string> titles{};
    
            // Now read all data and store vit in our resulting vector
            std::string tempTitle{};
            while ((fileStream >> tempTitle) and (tempTitle != "-1"))
                titles.push_back(tempTitle);
    
            // For debug purposes. Show all titles on screen:
            for (const std::string title : titles)
                std::cout << '\n' << title;
        }
        else std::cerr << "\n*** Error: Could not open file '" << fileName << "'\n";
    }