Read integers from coupled lines in a text file

Question

I'm trying to read a text file formatted as in this example:

5 3
3 1 5
6 6
6 1 3 2 5 4

Where the lines are coupled, with the line above always being of length equal to 2 and the line below having length equal to the the last digit in the line above.

(Unfortunately this is from an old note I took and I don't remember what those numbers are actually supposed to mean, so I'm limiting myself to store them in a multidimensional vector form where they can be easily recovered).

In the beginning I was tring to read the digit which represents the length below to execute a loop of finite length, but in the end this is the code I wrote:

#include <fstream>
#include <sstream>
#include <string>
#include <vector>
int main()
{
    std::string filname="data/file01.txt";
    std::ifstream file01(filname);
    std::vector<std::vector<int>> v;
    std::string line;
    while(getline(file01,line))
    {
        std::stringstream ls;
        ls<<line;
        int c;
        std::vector<int> t;
        while(!ls.eof())
        {
            ls>>c;
            t.push_back(c);
        }
        v.push_back(t);
    }
    return 0;
}

This gives me a 2-dimensional vector that can be easily accessed, but I don't like having to convert both the file and each string to a stream and I don't like using nested vectors.

Is there a way to achieve this goal with more simple and elegant code (even if it means to make the code more specific)?

Answer 1

I see a number of things that may help you improve your program.

Decompose your program into functions

All of the logic here is in main in one chunk of code. It would be better to decompose this into separate functions.

Don't loop on eof()

It's almost always incorrect to loop on eof() while reading a file. The reason is that the eof indication is only set when an attempt is made to read something from the file when we're already at the end. In this case, it means we have already executed the line ls >> c; and then called t.push_back(c);. The problem is that if we're at the end of the file and we attempt to read one more c value, it will append an unwanted value to the end of the vector. See this question for more details on why using eof is usually wrong.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Allow the user to specify input file name

The file name is currently hardcoded which certainly greatly restricts the usefulness of the program. Consider using argc and argv to allow the user to specify file names on the command line.

Use standard algorithms

An alternative is to use std::copy to read in a line of integers. Here's one way to do that:

while(getline(in, line))
{
    std::stringstream ls{line};
    std::vector<int> vec;
    std::copy(std::istream_iterator<int>(ls), 
              std::istream_iterator<int>(), 
              std::back_inserter(vec)
             );
    v.push_back(vec);
}

Use a custom object

An alternative would be to use your own class, LinePair and create a single-dimensional vector of those. Here's how the LinePair object might be defined:

#include <iostream>
#include <iterator>
#include <fstream>
#include <vector>

class LinePair {
public:
    friend std::ostream& operator<<(std::ostream& out, const LinePair& lp) {
        constexpr char sep[]{" "};
        auto it{lp.data.begin()};
        out << *it++ << sep << lp.data.size() - 1 << '\n';
        std::copy(it, lp.data.end(), 
                  std::ostream_iterator<int>(out, sep));
        return out << '\n';
    }
    friend std::istream& operator>>(std::istream& in, LinePair& lp) {
        int a, len;
        in >> a >> len;
        if (in && len) {
            lp.data.clear();
            lp.data.reserve(len+1);
            lp.data.push_back(a);
            for ( ; len && in >> a; --len) {
                lp.data.push_back(a);
            }
        }
        return in;
    }
private:
    std::vector<int> data;
};

Now main is quite simple:

int main(int argc, char *argv[])
{
    if (argc != 2) {
        std::cerr << "Usage: readlines filename\n";
        return 1;
    }
    std::ifstream in{argv[1]};
    std::vector<LinePair> v;
    std::copy(std::istream_iterator<LinePair>(in), 
              std::istream_iterator<LinePair>(),
              std::back_inserter(v)
             );

    // now echo it back out
    std::copy(v.begin(), v.end(), 
              std::ostream_iterator<LinePair>(std::cout));
}

Answer 2

You're turning the whole input file into a vector of vectors. That's fine, but you state yourself that your input has structure. It would be nice to have that structure reflected in your program:

1. since the lines are couple in pairs I'd like to see a code block that clearly processes a pair of lines
2. you should then eliminate the redundancy that the second number of the 1 st line equals the number of items on the second.