Search code examples
c++fileinputfile-ioio

Why does ifstream change its state when reading Doubles in C++?


I am writing a method that reads items from a file that is formatted like this:

[String] [Integer] [Double]

[String] [Integer] [Double]

I am reading from the file and storing each line into a vector as a user-defined type using the following loop:

 while(ifs.good()){
    Car car;
    ifs >> car.carName;
    ifs >> car.yearBuilt;
    if(!ifs.good()) throw std::invalid_argument("Year must be a integer");
    ifs >> car.price; 
    if(!ifs.good()) 
      throw std::invalid_argument("Price must be a double");
    to_return.push_back(car);
  }

However, despite a properly formatted file with the right fields, the function keeps throwing an error saying "Price must be a double". Here is a test file that I used:

Mercedes 1998 2364.12

Audi 2012 32645.79

If I delete the if statement, the file is parsed correctly and the vector works as intended. I am confused on why this error is being thrown and why the file state fixes itself if it is in the fail() state after parsing the double.


Solution

  • .good() is the wrong thing to check for streams a lot of the time, because it also checks whether the eof flag is set, i.e. whether the end of the file has been reached by some previous operation.

    Normally, reading a double wouldn't set the eof flag, because text files are supposed to end with a \n, so we would find a newline character instead of the EOF while reading. However, not every text file is properly terminated (including yours, apparently), and we run into the EOF while reading 2364.12.

    Ideally, your code should be able to handle text files that don't end in a newline character. If we do error handling correctly, this is possible. Most of the time, you should:

    1. try to perform some I/O operation
    2. check whether .fail() is set to see if it succeeded

    C++ streams can be contextually converted to bool, which checks the .fail() flag implicitly. As a result, we can write:

     // while(true) because at this point, we have done nothing yet
     // and don't have any loop condition we can check.
     // do-while loops also don't make sense when files are properly terminated.
     while(true) {
        Car car;
        if (!(ifs >> car.carName)) {
            // no more cars in stream,
            // we end the loop gracefully
            break;
        }
        if (!(ifs >> car.yearBuilt)) {
            // we've failed reading in the middle of the car's information
            // this is not okay
            throw std::invalid_argument("Failed to read year built");
        }
        if (!(ifs >> car.price)) {
            throw std::invalid_argument("Failed to read car price");
        }
        to_return.push_back(car);
    }