[C con Clase] Ayuda con Clase Modificar en C++

[Davidson, Steven]

Hola Guru,

2015-03-25 1:05 GMT-04:00 Ing CuCei <guru_wissens en outlook.com>:

> Buenos dias amigos soy nuevo en el grupo y programando fijence que tengo
> un codigo el cual quieren que le ponga la opcion de buscar que ya la tiene
> pero al momento de modificar un arreglo en x posicion lo que pasa es que me
> modifica el primer arreglo en vez que el seleccionado alguien me podria
> decir el problema Les dejo el codigo Muchas Gracias
>
>
Bienvenido al grupo y a este mundo de la programación.

Viendo un poco el código fuente que has escrito, hay varios matices a
destacar en cuanto al diseño. Voy a ir comentando el programa a medida que
lo vaya viendo.

//Hecho Por Guru_wissens
>
> #include<iostream>
> #define MAX 3
>
>
En C++, no es necesario usar constantes simbólicas. De hecho, se aconseja
usar constantes; esto es,

const int MAX = 3;

o incluso,

constexpr int MAX = 3;  // desde C++11

int ban=0;
> int cont_prod=0;
>

No es NADA aconsejable usar variables globales; sobre todo, si las usas en
las implementaciones de funciones miembro.

using namespace std;
> class Empleados
> {
>   public:
>
>
Los datos miembro no deberían ser públicos, sino privados.

int Id,Sal,i,contador;
> char cliente[25],Fec[10],RFC[25],Dir[30],ultc[10],Nss[20];
> long int Tel[3];
>

Aconsejo definir constantes para los tamaños de los arrays; por ejemplo,

public:
  const int CLIENTE_MAX = 25;  // permitido a partir de C++11
  const int FECHA_MAX = 10;  // permitido a partir de C++11
  ...

private:
  int Id, Sal;
  char cliente[CLIENTE_MAX], Fec[FECHA_MAX];
  ...

Además, aquí tienes algunas variables que no deberían pertenecer a la
representación interna de esta clase, como por ejemplo, 'i', que debería
ser local a cada función miembro; y luego tenemos 'contador' que no usas.

Por último, en general, no aconsejo representar un número de teléfono con
un tipo numérico. Algunos números requieren más dígitos que la cantidad
máxima que un tipo numérico pueda representar. Además, a veces se agregan
otros símbolos junto con los dígitos de un número de teléfono. Por estas
razones, recomiendo representar un teléfono como una cadena de caracteres.

    public:
>
>
>
Te falta crear constructores para esta clase. Aconsejo crear al menos el
constructor sin parámetros:

Empleados();

    void capturar();
>     void mostrar();
> void buscar();
> void modificar(int x);
> }e;
>
>
Instancias este objeto globalmente, pero no lo usas; deberías eliminarlo.

Empleados em[10];
>
>
Nuevamente, no es nada recomendado crear variables globales. Este array
debería existir localmente en 'main()'.

void Empleados::capturar()
> {
>   cout<<"\t\nID:" <<endl;
>   cin>>Id;
>

Ten cuidado al leer diferentes tipos de datos, especialmente cadenas de
caracteres y números. En este caso, lees un entero al que sigue la lectura
de una cadena de caracteres. El problema es que en el canal entrante,
'cin', existe el carácter '\n', por lo que al leer una cadena, no pregunta
al usuario porque el canal no está vacía, al leer y extraer el carácter,
'\n', de la lectura previa. Esto implica que el programa parece saltarse la
lectura de 'Nss'.

La solución es extraer todos los caracteres posteriores a la lectura
anterior del entero. Puedes sacar el carácter, '\n', así,

cin.get();  // para extraer '\n'

Si crees que pudiere haber más, porque al usuario le importó poco ser
"amable" con tu programa, entonces invoca 'ignore()'; por ejemplo,

cin.ignore();


[CORTE]

void Empleados::buscar()
> {
>     int Clave;
> cout<<"\nID del Cliente:"<<endl;
>     cin>>Clave;
> if(Id==Clave)
>    {
>    cout<<"\nEmpleado:"<<cliente;
>    cout<<"\nID: "<<Id;
>    cout<<"\nNss: "<<Nss;
>    cout<<"\nR.F.C: "<<RFC;
>    cout<<"\nPuesto: "<<ultc;
>    for(i=0;i<3;i++)
>            {
>              cout<<"\t\nTelefono:"<<i+1<<Tel[i] <<endl;
>
>
>            }
>    cout<<"\nDireccion: "<<Dir;
>    cout<<"\nFecha: "<<Fec;
>
>
>        }
>
> }
>
>
Esta función solamente muestra los datos guardados si la clave concuerda
con la ID de un registro. Sin embargo, esto realmente no es buscar un
empleado, porque esta función sólo sirve para un empleado en particular, y
no en una lista de empleados. Esto tiene que ver con el diseño que has
realizado. Hablo de esto al final del correo-e.

void Empleados :: modificar(int x)
> {
>     int opc=0;
> int tam=4;
>
>
En primer lugar, no necesitas que 'tam' sea una variable, sino una
constante. En segundo lugar, 'tam' representa la cantidad de elementos en
el array, 'em', que obviamente no es 4, sino 10.

He aquí la razón de usar constantes en lugar de usar el valor directamente.

for(int j=0;j<tam;j++)
> {
>
> if(x==Id)
> {
> cout<<"\nEl Codigo es: \n";
>             mostrar();
> while(opc!=5)
> {
> cout<<"\n\nQue deseas modificar "<<endl;
> cout<<"\n1. Codigo "<<endl;
> cout<<"\n2. Nombre "<<endl;
> cout<<"\n3. Fecha "<<endl;
> cout<<"\n4. Nss "<<endl;
> cout<<"\n5. Ir a menu de servicio"<<endl;
>                 cin>>opc;
>
>
Cada función debe realizar la tarea imputada; ni más ni menos. En esta
función, 'modificar()' debería realizar tal tarea: modificar un objeto de
'Empleados'. Sin embargo, esta función realiza varias tareas: modificar un
miembro y comunicarse con el usuario. No es la responsabilidad de esta
función comunicarse con el usuario, por lo que no debería estar aquí.

                switch (opc){
>                     case 1: cout<<"\nCodigo: ";
> cin>>em[j].Id;
>

Esto no es NADA aconsejable. Básicamente, cualquier objeto de 'Empleados'
accederá a un array global de objetos de 'Empleados'.

Al final del correo-e hablo del diseño y rediseño de este programa.

                        break;
>                     case 2:cin.ignore(100,'\n');
>                         cout<<"\nNombre: ";
> cin.getline(em[j].cliente,20,'\n');
>

Debería ser 25, porque 'cliente' ocupa 25 caracteres. Nuevamente, el uso de
constantes soluciona este problema de usar diferentes valores cuando
deberían ser el mismo valor.

                        break;
> case 3: cout<<"\nFecha: ";
> cin>>em[j].Fec;
> break;
> case 4: cout<<"\nNss: ";
> cin>>em[j].Nss;
> break;
>  }
>
> }
> }
> else ban=1;
>

Aquí te comunicas con la variable global, 'ban'. Si necesitas comunicación
con el exterior a modo de resultado, entonces usa el mecanismo de las
funciones y retorno de valores usando el vocablo, 'return'. Esto es,

int Empleados::modificar( int x )
{
  ...
  return resultado;   // antes era 'ban'
}

Ya la hora de invocar esta función, obtendríamos el valor retornado. Por
ejemplo,

for( int x=1; x<Idx; x++ )
  ban = em[x].modificar( Idx );

if( 1 == ban )
{
  ...
}

Por otro lado, parece que 'ban' sólo se usa para guardar el valor 0 (cero)
o 1. Como es binario, seguramente se beneficiaría de un tipo booleano; por
ejemplo,

bool Empleados::modificar( int x )
{
  ...
  return resultado;   // antes era 'ban'
}
...

bool ban = false;
...
for( int x=1; x<Idx; x++ )
  ban = em[x].modificar( Idx );

if( ban )
{
  ...
}

}
>
> };
>
>
Este punto y coma no es necesario; puedes eliminarlo.


>

> int main()
> {
>

[CORTE]

 }
>

Debes retornar un entero, que por convenio, 0 (cero) indica una terminación
exitosa.


El diseño que has realizado no es el más apto a las necesidades del
problema que se te haya dado. Por lo que veo, necesitas crear una lista de
empleados. Esto implica que tenemos dos conceptos importantes:
1. Empleado, y
2. Lista de Empleados

Por lo tanto, interesa representar cada idea compleja con una clase, como
por ejemplo,

class Empleado
{
  ...
};

class ListaEmpleado
{
  ...
};

Para poder representar cada idea, tienes que diseñar la representación
interna de cada idea a nivel de datos al igual que sus operaciones y tareas
que ofrecen para comunicarse con el exterior: otras ideas, funciones, y el
programa (principal). Por ejemplo,

class Empleado
{
public:
  const int MAX_CLIENTE = 25;
  const int MAX_FECHA = 10;
  ...

private:
  int nID;
  float fSalario;  // creo que "salario" debería ser 'float'
  char szCliente[MAX_CLIENTE];
  ...

public:
  Empleado();
  Empleado( int id, float salario, char cliente[], ... );

  // Get y Set
  int getID() const   { return nID; }
  void setID( int id )  { nID = id; };

  float getSalario() const   { return nSalario; }
  void setSalario( float salario )  { nSalario = salario; };
  ...
};

Como puedes ver, no tiene las funciones miembro de 'capturar()',
'mostrar()', etc. porque no definen las operaciones básicas para manipular
objetos de la clase 'Empleado'. Sí puedes crear funciones globales que
realicen estas tareas más elevadas (menos básicas) como funciones
auxiliares; por ejemplo,

void mostrar( const Empleado &e )
{
  cout << "ID: " << e.getID() << endl;
  cout << "Nss: " << e.getNSS() << endl;
  cout << "Cliente: " << e.getCliente() << endl;
  ...
}

Lo mismo sucede con otras funciones como 'capturar()'.

Tenemos que hacer lo mismo que con 'Empleado' pero con 'ListaEmpleados';
por ejemplo,

class ListaEmpleados
{
public:
  const int MAX_EMPLEADOS = 10;

private:
  Empleado lista[MAX_EMPLEADOS];

public:
  ListaEmpleados();

  // Leer: 1 empleado
  const Empleado & getEmpleado( int n ) const;

  // Modificar: 1 empleado
  void modificar( int n );

  // Buscar: 1 empleado => índice al Empleado encontrado
  int buscar( int id ) const;
};

Esto es una forma bastante más organizada de diseñar, desglosando los
conceptos importantes que existen para componer un programa.


Espero que esto te oriente.

Steven
------------ próxima parte ------------
Se ha borrado un adjunto en formato HTML...
URL: <http://listas.conclase.net/pipermail/cconclase_listas.conclase.net/attachments/20150331/c559e17b/attachment.html>