One extra loop when reading struct from file into a vector

[anarelle]

I have this struct with 2 strings and a long, which I read from keyboard and save into a text file like this:

struct libro
{
	string nombre;
	string genero;
	long isbn;
};


void save_libro(const string& file_name, libro& un_libro)
{
	ofstream archivo(file_name, ios::app);
	archivo << un_libro.nombre << endl;
	archivo << un_libro.genero << endl;
	archivo << un_libro.isbn << endl;
	archivo.close();
}


void agregar_libro()
{
	libro un_libro;
	cout << "\n\n\nAGREGAR LIBRO \n" << endl;
	cout << "Nombre: ";
	getline(cin, un_libro.nombre);
	cout << "Genero: ";
	getline(cin, un_libro.genero);
	cout << "ISBN: ";
	cin >> un_libro.isbn;
        
	save_libro("libros.txt", un_libro);   //save to file
}

My file is correctly saved. This is how it looks after I saved 3 test structs to it:

book example 1
book genre 1
1111
book example 2
book genre 2
2222
book example 3
book genre 3
3333

I noticed there is an extra line added at the end of the file.

Then at some point I need to read all of that into structs that I save in a vector:

void read_libros(const string& file_name, vector<libro>& libros)
{
	libro un_libro;
	ifstream archivo(file_name);
	if (!archivo.is_open())
	{
		cout << "File can't be opened." << endl;
	}
	else
	{
		while (!archivo.eof())
		{
			getline(archivo, un_libro.nombre);
			getline(archivo, un_libro.genero);
			archivo >> un_libro.isbn;
			archivo.ignore();
			libros.push_back(un_libro);
		}
		archivo.close();
	}
}


void modificar_libro()
{
	vector<libro> mis_libros;
	read_libros("libros.txt", mis_libros);
}

Everything works fine, except for the fact that the vector is loaded with one extra item. My Visual Studio 2013 debugger shows me that, after executing the function "modificar_libro()" the vector has the following items:

[0] {nombre="book example 1" genero="book genre 1" isbn=1111 } libro
[1] {nombre="book example 2" genero="book genre 2" isbn=2222 } libro
[2] {nombre="book example 3" genero="book genre 3" isbn=3333 } libro
[3] {nombre="" genero="book genre 3" isbn=3333 } libro

I suspect the extra line saved at the end of the file could have something to do with the issue, but I'm not sure how to fix it.

[AbstractionAnon]

The problem is line 11. After you read the last record, eof is NOT set. eof is only set when you attempt a read and there is nothing to read. When you subsequently do the getline at line 13-14, the eof bit is set, but you don't check at that point so you proceed to do the push_back() as if the getlines had succeeded.

[LB]

Actually, it is considered a mistake to not have a blank line at the end of a plain text file.

The issue is that you are looping on .eof(), which is incorrect. The EOF flag is not set until after a read operation fails, so even if you are at the end of the file, the EOF flag is not set until you try to read something and fail. Do not loop on EOF.

Instead, loop on the input operation:

		while(getline(archivo, un_libro.nombre)
		   && getline(archivo, un_libro.genero)
		   && getline(archivo, un_libro.genero)
		   && (archivo >> un_libro.isbn)
		   && archivo.ignore()
		)
		{

[anarelle]

Thanks so much! It's working wonderfully now.
But I'm a bit confused: according to C++ docs (http://www.cplusplus.com/reference/string/string/getline/) getline returns a istream& and not a boolean value, and the same goes for ignore(). Why do they work as a condition on a while loop then?
Thanks again :)

[AbstractionAnon]

Why do they work as a condition on a while loop then?

Because streams have an overload for operator void * () (C++98) and operator bool() (C++11). Those overloads can be used in condition statements.