Error reading and printing a text file with C++

Question

I have a bug with my code (the code at the end of the question). The purpose of my C++ executable is to read a file that contains numbers, copy it in a std::vector and then just print the contents in the stdout? Where is the problem? (atoi?)

I have a simple text file that contains the following numbers (each line has one number)

mini01:algorithms ios$ cat numbers.txt 
1
2
3
4
5

When I execute the program I receive one more line:

mini01:algorithms ios$ ./a.out 
1
2
3
4
5
0

Why I get the 6th line in the stdout?

#include <iostream>
#include <string>
#include <fstream>
#include <vector>
using namespace std;

void algorithm(std::vector<int>& v) {

    for(int i=0; i < v.size(); i++) {
        cout << v[i] << endl;
    }

}

int main(int argc, char **argv) {

    string line;
    std::vector<int> vector1;
    ifstream myfile("numbers.txt");
    if ( myfile.is_open()) {
        while( myfile.good() )
        {
            getline(myfile, line);
            vector1.push_back(atoi(line.c_str()));
        }
        myfile.close();
    }
    else {
        cout << "Unable to open file" << endl;
    }

    algorithm(vector1);

    return 0;

}

Answer 1

You should not use while (myfile.good()), as it will loop once to many.

Instead use

while (getline(...))

The reason you can't use the flags to check for looping, is that they don't get set until after an input/output operation notices the problem (error or end-of-file).

Answer 2

Don't use good() as the condition of your extraction loop. It does not accurately indicate whether the next read will succeed or not. Move your call to getline into the condition:

while(getline(myfile, line))
{
    vector1.push_back(atoi(line.c_str()));
}

The reason it is failing in this particular case is because text files typically have an \n at the end of the file (that is not shown by text editors). When the last line is read, this \n is extracted from the stream. Yes, that may be the very last character in the file, but getline doesn't care to look any further than the \n it has extracted. It's done. It does not set the EOF flag or do anything else to cause good() to return false.

So at the next iteration, good() is still true, the loop continues and getline attempts to extract from the file. However, now there's nothing left to extract and you just get line set to an empty string. This then gets converted to an int and pushed into the vector1, giving you the extra value.

In fact, the only robust way to check if there is a problem with extraction is to check the stream's status bits after extracting. The easiest way to do this is to make the extraction itself the condition.

Answer 3

You read one too many lines, since the condition while is false AFTER you had a "bad read".

Answer 4

Welcome to the wonderful world of C++. Before we go to the bug first, I would advise you to drop the std:: namespace resolution before defining or declaring a vector as you already have

using namespace::std;

A second advise would be to use the pre increment operator ++i instead of i++ wherever feasible. You can see more details on that here.

Coming to your problem in itself, the issue is an empty new line being read at the end of file. A simple way to avoid this would be to check the length of line before using it.

getline(myfile, line);
if (line.size()) {
  vector1.push_back(atoi(line.c_str()));
}

This would enable your program now to read a file interspersed with empty lines. To be further foolproof you can check the line read for presence of any non numeric characters before using atoi on it. However the best solution as mentioned would be use to read the line read to the loop evaluation.