Question:
I have a little problem with an exercise in a (proto) employee database, which consists of creating a console application to store data entered by the user in a vector of elements of a class "Employee". This class is abstract and from it three others called "Boss", "Engineer" and "Researcher" inherit and all of them are very simple, they have character arrays as members only to store the data (name, salary, experience …) .
When I say that they are simple, I mean that there are no pointers or vectors or anything complex within them. The program should have the option to save the data in a binary file and be able to load that data back when you run the program again.
Everything works correctly, but when trying to visualize the data once loaded from the binary file the program "casca" and I don't understand why.
This is the parent class 'Employee':
class Empleado
{
public:
virtual ~Empleado() {} ;
virtual void MostrarDatos() = 0;
protected:
char mTipoEmpleado[16];
char mNombre[100];
char mApellido[100];
float mSalario;
private:
};
This is one of the classes that inherits from Employee, in this case it is the 'Boss' class:
class Jefe : public Empleado
{
public:
Jefe();
Jefe(char Nombre[], char Apellido[], float Salario, unsigned int reunionesSemanales, unsigned int diasVacacionesAnuales);
virtual ~Jefe() {}
void MostrarDatos();
protected:
unsigned int mReunionesSemanales;
unsigned int mDiasVacacionesAnuales;
private:
};
This is the Main function:
int main(int argc, char** argv) {
cout << endl << " ///////////////////////////////////////////////////////////" << endl;
cout << " // BIENVENIDO AL GESTOR DE LA BASE DE DATOS DE EMPLEADOS //" << endl;
cout << " // //" << endl;
cout << " ///////////////////////////////////////////////////////////" << endl;
int eleccion = 0;
bool salir = false;
vector<Empleado*> empleados;
while (!salir)
{
cout << endl << " Escoge una opcion por favor : " << endl;
cout << endl << " 1.- Agregar empleado" << endl;
cout << " 2.- Eliminar empleado" << endl;
cout << " 3.- Guardar base de datos" << endl;
cout << " 4.- Cargar base de datos" << endl;
cout << " 5.- Mostrar base de datos" << endl;
cout << " 6.- Salir" << endl;
cout << " Opcion : ";
cin >> eleccion;
switch (eleccion)
{
case 1:
{
system("cls");
bool salirTipoEmpleado = false;
int tipoEmpleado = 0;
while (!salirTipoEmpleado)
{
cout << endl << " Escoge tipo de empleado por favor : " << "\n";
cout << endl << " 1.- Jefe" << "\n";
cout << " 2.- Ingeniero" << "\n";
cout << " 3.- Investigador" << "\n";
cout << " 4.- Volver" << "\n";
cout << " Opcion : ";
cin >> tipoEmpleado;
switch (tipoEmpleado)
{
case 1:
{
empleados.push_back(new Jefe);
system("cls");
cout << " Nuevo Jefe incluido con exito en la base de datos." << "\n";
}
break;
case 2:
{
empleados.push_back(new Ingeniero);
system("cls");
cout << " Nuevo Ingeniero incluido con exito en la base de datos." << "\n";
}
break;
case 3:
{
empleados.push_back(new Investigador);
system("cls");
cout << " Nuevo Investigador incluido con exito en la base de datos." << "\n";
}
break;
case 4:
salirTipoEmpleado = true;
break;
system("cls");
} // Fin switch
} // Fin while(!salirTipo...)
} // Fin case 1
break;
case 2:
{
system("cls");
if (empleados.size() == 0)
cout << " Base de datos vacia." << "\n";
else
{
unsigned int indice = -1;
while (indice < 0 || indice >= empleados.size())
{
cout << " La base de datos tiene " << empleados.size() << " empleados." << "\n";
cout << " Elige empleado a borrar (primer empleado es el numero 0) : ";
cin >> indice;
if (indice < 0 || indice >= empleados.size()) cout << " Fuera de rango." << "\n";
else
{
empleados.erase(empleados.begin() + indice);
cout << " Empleado " << indice << " borrado." << "\n";
indice = 0;
if (empleados.size() == 0) break;
}
}
}
}
break;
case 3:
{
ofstream guardarDatos("Base_datos_Empleados.dat", ios::binary);
GuardarDatosBinario("Base_datos_Empleados.dat", empleados);
if (guardarDatos.good()) cout << " Base de datos guardada correctamente." << "\n";
else cout << " Error al guardar Base de datos." << "\n";
}
break;
case 4:
{
ifstream cargarDatos("Base_datos_Empleados.dat", ios::in | ios::binary | ios::ate);
if (cargarDatos.is_open())
{
empleados = LeerArchivoBinario("Base_datos_Empleados.dat");
}
else cout << " Error al cargar datos." << "\n";
}
break;
case 5:
{
system("cls");
if (empleados.size() == 0)
{
cout << " Base de datos vacia.";
break;
}
else cout << " La base de datos tiene " << empleados.size() << " empleados. " << "\n";
for (Empleado* emp : empleados)
{
emp->MostrarDatos();
cout << endl;
}
}
break;
case 6:
{
empleados.erase(empleados.begin(), empleados.end());
if (empleados.size() == 0)
cout << " Base de datos borrada. " << "\n";
else cout << " Fallo al borrar la base de datos. " << " \n";
} // Fin case 5
salir = true;
break;
} // Fin switch(eleccion)
} // Fin while(!salir)
return 0;
}
This is the function that saves the data in a binary file. It takes as arguments a file name and a vector of pointers to 'Employee':
void GuardarDatosBinario(const char* archivoBinario, const vector<Empleado*> empleados)
{
ofstream guardarDatos(archivoBinario, ios::binary);
for (const Empleado* emp : empleados)
guardarDatos.write(reinterpret_cast<char*>(&emp), sizeof(emp));
}
And this is the one that loads the data from the binary file into the vector of pointers to Employees:
vector<Empleado*> LeerArchivoBinario(const char* archivoBinario)
{
vector<Empleado*> temporal;
ifstream cargarDatos(archivoBinario, ios::binary);
Empleado* empleadoTemp;
while (cargarDatos.read(reinterpret_cast<char*>(&empleadoTemp),sizeof(empleadoTemp)))
temporal.push_back(empleadoTemp);
return temporal;
}
Apparently the data is loaded because this line
cout << " La base de datos tiene " << empleados.size() << " empleados. " << "\n";
included just before the loop to visualize the data of the vector of pointers 'Employee' answers the correct number (2, 3, 7, 10 …) but immediately after when starting to traverse the vector, the dreaded "segmentation fault" appears.
This is the loop
for (Empleado* emp : empleados)
{
emp->MostrarDatos(); //-----> AQUÍ APARECE EL ERROR.
cout << endl;
}
And this is the function to display the data, in this case of the class 'Boss' but the others are the same:
void Jefe::MostrarDatos()
{
std::cout << " Tipo empleado : " << mTipoEmpleado << "\n";
std::cout << " Nombre : " << mNombre << "\n";
std::cout << " Apellido : " << mApellido << "\n";
std::cout << " Salario : " << mSalario << "\n";
std::cout << " Reuniones Semanales : " << mReunionesSemanales << "\n";
std::cout << " Dias de vacaciones anuales : " << mDiasVacacionesAnuales << "\n";
}
Anyway, I don't understand why it doesn't work. I wonder if it has something to do with the fact that an abstract class being 'Employee' cannot be 'saved' so easily in a binary file due to the 'vtable' and that you have to use serialization or something like that, which for now is science fiction for me.
Any guidance on this is more than welcome.
Answer:
void GuardarDatosBinario(const char* archivoBinario, const vector<Empleado*> empleados)
{
ofstream guardarDatos(archivoBinario, ios::binary);
for (const Empleado* emp : empleados)
guardarDatos.write(reinterpret_cast<char*>(&emp), sizeof(emp));
}
A silly detail of the above code is that you should avoid passing large objects by value, since this means making a copy of it. In your case it is usually convenient to use a constant reference:
void GuardarDatosBinario(const char* archivoBinario, const vector<Empleado*>& empleados)
// ^
{
ofstream guardarDatos(archivoBinario, ios::binary);
for (const Empleado* emp : empleados)
guardarDatos.write(reinterpret_cast<char*>(&emp), sizeof(emp));
}
In any case, this method will not work because emp is a pointer, so &emp is a double pointer and sizeof(emp) will return the size of a pointer (4 bytes in 32bits and 8 bytes in 64). What the program is doing at this point is, basically, storing in the file the memory address where the object is currently located … but rest assured that it will not find it there the next time you read the file. To store the data you would have to store the memory pointed to by emp … but as you are about to discover it will give you new problems. For now we can assume that you leave the function like this:
void GuardarDatosBinario(const char* archivoBinario, const vector<Empleado*>& empleados)
{
ofstream guardarDatos(archivoBinario, ios::binary);
for (const Empleado* emp : empleados)
guardarDatos.write(reinterpret_cast<char*>(emp), sizeof(*emp));
}
To explain one of the problems with this new version of the function, take a look at the following example:
int main(int argc, char** argv) {
std::cout << sizeof(Empleado);
}
What value would you say the program will print?
A quick glance would tell us that: 16 Bytes ( mTipoEmpleado ) + 100 Bytes ( mNombre ) + 100 Bytes ( mApellido ) + 4 Bytes ( mSalario ) = 220 Bytes
But if I run the above code the program prints the following:
224
Where do those extra 4 bytes come from? Those 4 extra Bytes are occupied by an internal pointer of the class that points to a structure that is used to handle virtual calls and polymorphism (known as vtable ). It does not seem very sensible to write that memory address in the file because when you try to retrieve the information from the file that pointer will no longer be valid and the program will give you strange errors.
Another problem with this function is that sizeof is not able to read the vtable , so it does not know if the pointer is of an object of type Jefe (for example) and, consequently, it will always return the size of the Empleado class . The effect achieved is that only the information of the base class will be stored, losing the rest.
The third problem with this function is that if in the future you char mNombre[100] change, for example, char mNombre[100] to string mNombre , it will be impossible to both recover old data and save new ones. std::string as well as many other classes usually make use of dynamic memory and that will cause the file to store memory addresses instead of valid data.
Due to the nature of cases in C ++, to serialize information in a file, the best practice is to write the data explicitly:
void GuardarDatosBinario(const char* archivoBinario, const vector<Empleado*> empleados)
{
ofstream guardarDatos(archivoBinario, ios::binary);
for (const Empleado* emp : empleados)
{
guardarDatos.write(emp->mTipoEmpleado,sizeof(emp->mTipoEmpleado));
guardarDatos.write(emp->mNombre,sizeof(emp->mNombre));
guardarDatos.write(emp->mApellido,sizeof(emp->mApellido));
guardarDatos.write((char*)&emp->mSalario,sizeof(emp->mSalario));
}
}
Note that in order to access the members of the class you will have to declare them as public or declare the function as an Empleado friend . You can also create a set of getters to provide access to the data.
Note that even so at this point you are only saving the data from the base class. How should the derived classes be saved? You could make the function be able to recognize each of the objects, but it is not recommended because the function will start to be complicated to manage:
void GuardarDatosBinario(const char* archivoBinario, const vector<Empleado*> empleados)
{
ofstream guardarDatos(archivoBinario, ios::binary);
for (const Empleado* emp : empleados)
{
guardarDatos.write(emp->mTipoEmpleado,sizeof(emp->mTipoEmpleado));
guardarDatos.write(emp->mNombre,sizeof(emp->mNombre));
guardarDatos.write(emp->mApellido,sizeof(emp->mApellido));
guardarDatos.write((char*)&emp->mSalario,sizeof(float));
if( Jeje* jefe = dynamic_cast<Jefe*>(emp) )
{
guardarDatos.write((char*)emp->mReunionesSemanales,sizeof(int));
// ...
}
else if( // ...
}
}
Another possibility would be that each object knew how to save itself (with a virtual function), but I don't know if this solution is valid for your exercise because I ignore the restrictions.
Well, the save is finished, right? Well no. Missing? let's see it from the perspective of reading. We mTipoEmpleado to read the first record of the file ( mTipoEmpleado , mNombre , mApellido , mSalario ) and now we find more information … Is it a new record? Is it data from an object of type Jefe ? Or will it be information from an Ingeniero type object? Not only do you not have information to know what decision to make, but you should also have made the decision before: You should know what type of object you have to create before you start reading the registry data).
What is missing in the file is a mark or identification that allows you to know what type of record you are going to read. This mark could be a simple enumerated:
enum TipoEmpleado
{
Indefinido,
Jefe,
Ingeniero,
Investigador,
};
Then the save could be like this (for cleaning I have divided the function):
void Write(std::ofstream& out, int valor)
{
out.write((char*)&valor,sizeof(int));
}
void Write(std::ofstream& out, float valor)
{
out.write((char*)&valor,sizeof(float));
}
void GuardarDatosEmpleado(std::ofstream& out, const Empleado& emp)
{
guardarDatos.write(emp.mTipoEmpleado,sizeof(emp.mTipoEmpleado));
guardarDatos.write(emp.mNombre,sizeof(emp.mNombre));
guardarDatos.write(emp.mApellido,sizeof(emp.mApellido));
Write(guardarDatos,emp.mSalario);
}
void GuardarDatosBinario(const char* archivoBinario, const vector<Empleado*> empleados)
{
ofstream guardarDatos(archivoBinario, ios::binary);
for (const Empleado* emp : empleados)
{
if( Jeje* jefe = dynamic_cast<Jefe*>(emp) )
{
Write(guardarDatos,TipoEmpleado::Jefe);
GuardarDatosEmpleado(*emp);
Write(guardarDatos,jefe->mReunionesSemanales);
Write(guardarDatos,jefe->mDiasVacacionesAnuales);
}
else if( Ingeniero* ingeniero = dynamic_cast<Ingeniero*>(emp) )
{
Write(guardarDatos,TipoEmpleado::Ingeniero);
GuardarDatosEmpleado(*emp);
// ...
}
}
}
Let's now review the read function:
vector<Empleado*> LeerArchivoBinario(const char* archivoBinario)
{
vector<Empleado*> temporal;
ifstream cargarDatos(archivoBinario, ios::binary);
Empleado* empleadoTemp;
while (cargarDatos.read(reinterpret_cast<char*>(&empleadoTemp),sizeof(empleadoTemp)))
temporal.push_back(empleadoTemp);
return temporal;
}
Look at this line:
Empleado* empleadoTemp;
You declare a pointer but you do not reserve memory for it, then it will point to a random position and the Operating System does not usually like you to access memory that does not correspond to you.
Here you will have to create an object of type Jefe , Ingeniero or Investigador . How to do it? We have already solved part of the problem by reviewing the save. The integer that precedes the record will tell us what type of object is being stored next:
Empleado* empleadoTemp = 0; // Es importante inicializar los punteros
TipoEmpleado tipo;
cargarDatos.Read((char*)&tipo,sizeof(int));
switch( tipo )
{
case TipoEmpleado::Jefe:
empleadoTemp = new Jefe;
// Rutina para leer un registro de tipo Jefe
break;
case TipoEmpleado::Ingeniero:
empleadoTemp = new Ingeniero;
// Rutina para leer un registro de tipo Ingeniero
break;
}
And of course we remove the line:
while (cargarDatos.read(reinterpret_cast<char*>(&empleadoTemp),sizeof(empleadoTemp)))
Keep in mind that now you will have to read each variable individually, something similar to what you have seen in writing:
cargarDatos.read(empleadoTemp->mTipoEmpleado,sizeof(empleadoTemp->mTipoEmpleado));
Regarding each object knowing how to save itself with a virtual function, it was something that I thought at first but I discarded because it seemed to me that it would duplicate a lot of code …
If you override a function in a derived class you can force calls to the parent class's function to reuse its code:
struct A
{
virtual void func()
{ std::cout << "A::func()" << std::endl; }
};
struct B : A
{
void func()
{
std::cout << "B::func()" << std::endl;
A::func();
}
};
int main()
{
B b;
b.func();
}
This call can be made anywhere, although it is usual to find it at the beginning or at the end of the derived function.
In your specific case, the saving process could be done easily without duplicating code:
class Empleado
{
public:
virtual void Guardar(std::ofstream& out)
{
guardarDatos.write(mTipoEmpleado,sizeof(mTipoEmpleado));
guardarDatos.write(mNombre,sizeof(mNombre));
guardarDatos.write(mApellido,sizeof(mApellido));
Write(guardarDatos,mSalario);
}
};
class Jefe : public Empleado
{
void Guardar(std::ofstream& out)
{
Write(guardarDatos,TipoEmpleado::Jefe);
Empleado::Guardar(out);
Write(guardarDatos,jefe->mReunionesSemanales);
Write(guardarDatos,jefe->mDiasVacacionesAnuales);
}
};