Is this the right way to read every line?

Question

I am never to sure when it comes to ifstream and reading lines. I am often confused with the good(), bad(), eof() and so on.

Could anyone tell me if I am doing it right?

int parseLine(std::ifstream & _file)
{  

  while( std::getline( _file, line, '\n' ).good() && !_file.eof() )
  {
     // some treatment
  }

  return 0;
}

Answer 1

Too complicated. Just say:

for (std::string line; std::getline(_file, line); )
{
    // process "line"
}

The function std::getline, like most iostreams operations, returns a reference to the stream object itself, which can be evaluated in a boolean context to tell you whether it is still good, i.e. whether the extraction operation succeeded.

Also, the function shouldn't be called "parseLine", but "parseAllLines" or "parseStream".

Answer 2

I prefer:

void parseLine(std::ifstream& file)
{  
    std::string line;
    while(std::getline(file, line))
    {
       // some treatment
    }
}

Comments on your code:

int parseLine(std::ifstream & _file)
                        //   ^^^  be careful with identifiers that start with _

Do you know all the rules? They are non trivial so best to just avoid '_' as a leading character.

This while loop does not buy you anything.

while( std::getline( _file, line, '\n' ).good() && !_file.eof() )

The result of std::getline() is a reference to a stream. When used in a boolean context it will be converted to a bool automatically (using the cast operator). This conversion will call good() so there is no point in calling it manually.

If the file is good() then eof() will not fail.

If you always return the same value

return 0;

Then why have a return value.

One thing about code it should be obvious what the code is doing with the need for extra comments. Comments mean the code is complex and needs additional explanation. But to make the code easy to read you should also use identifiers that convey some meaning.

void parseLine() // looks like it will read one line.

The term parseLine() is actually misleading here. As you don't parse a line. You parse the whole file.

Answer 3

At least IMO, the best way to read all the lines from a file is to start with a small proxy class:

class line { 
    std::string data;
public:
    operator std::string() { return data; }
    friend std::istream &operator>>(std::istream &is, line &d) { 
        return std::getline(is, d.data);
    }
};

Then read your lines with an istream_iterator, such as:

std::vector<std::string> lines((std::istream_iterator<line>(infile)),
                                std::istream_iterator<line>());

Or:

std::transform(std::istream_iterator<line>(infile),
               std::istream_iterator<line>(),
               some_output_iterator,
               some_function);