Ver Mensaje Individual
  #6 (permalink)  
Antiguo 04/06/2015, 15:16
eferion
 
Fecha de Ingreso: octubre-2014
Ubicación: Madrid
Mensajes: 1.212
Antigüedad: 9 años, 7 meses
Puntos: 204
Respuesta: Programa para consultar datos struct

Te enumero las cosas que veo:

El buffer hay que limpiarlo antes de leer

Código C++:
Ver original
  1. //Mal
  2. cin>>option;
  3. cin.ignore(256,'\n');
  4.  
  5. //Bien
  6. cin.ignore(256,'\n');
  7. cin>>option;

Se supone que tu intención es leer el dato que el usuario vaya a introducir en ese momento... no basura que ya esté almacenada en el buffer de entrada. Lo más normal es, primero limpiar el buffer y después realizar la lectura... como la lectura es bloqueante y el buffer estará vacío, el programa se quedará esperando a que el usuario introduzca un dato.

Las estructuras es mejor definirlas fuera de las funciones.

Si la estructura la defines dentro de una función, únicamente podrás crear variables de ese tipo dentro de la función, fuera de esa función la estructura, simplemente, no existirá.

Código C++:
Ver original
  1. struct oficina{
  2.     int numero;
  3.     long CP, tlf;
  4.     string nombre;
  5.     string ciudad;
  6.     string direccion;
  7.     string ciudad2;
  8.     string municipio;
  9. };
  10.  
  11.  
  12. main()
  13. {

Respeta la firma del main

La firma (la declaración) del main debería tener una forma tal que:

Código C++:
Ver original
  1. int main( int, char** )

Es altamente recomendable respetar esta firma por varias razones:
  • Las funciones SIEMPRE han de indicar el valor de retorno. El main no debería ser una excepción
  • El valor de retorno del main puede ser aprovechado por otros programas para saber si tu programa se ha ejecutado correctamente
  • Si no indicas los argumentos te va a resultar imposible arrancar tu aplicación con parámetros

Igualmente, lo lógico es acabar el main con el return correspondiente. La forma más correcta de indicar ese return es:

Código C++:
Ver original
  1. return EXIT_SUCCESS;

EXIT_SUCCESS es una constante definida en la librería stdlib.h. Como te comenté en una respuesta anterior, hay que intentar no poner valores "a pelo" en el código.

Cuidado con el cin.ignore

Cuando diseñas un programa tienes que intentar pensar que lo va a usar un mono. ¿Por qué? porque no puedes dar nada por sentado. No debes esperar que el usuario siempre te responda lo que tú quieres:
  • Si le pides un número te puede introducir un texto
  • Si le pides una opción entre la 'a' y la 'd' el usuario puede introducir la 'j'
  • El usuario puede hacer uso del teclado incluso cuando no se lo estés pidiendo, llenando el buffer de entrada de guarrería
  • ...

Atendiendo al tercer punto, si por la razón que sea el buffer del teclado tiene más de 256 caracteres la limpieza que has programado simplemente no será suficiente. En situaciones normales no pasará nada, pero debes tenerlo en cuenta.

Comparaciones

Código C++:
Ver original
  1. for(int z=0; z <strlen(localidad)-1; z++)
  2. { //for de comparacion
  3.   if((localidad[z] == lista[i].ciudad[z])||(localidad[z] == lista[i].ciudad2[z]))

Si tu idea es comparar dos variables de tipo char*, es preferible usar la función strcmp. Como norma general, si una función de la librería estándar hace lo que necesitas es preferible usarla. Motivos:
  • La STL será, en el peor de los casos tan óptima como tu solución. Lo lógico es que tienda a ser más óptima.
  • Reduces la cantidad de código a de tu programa, lo que reduce el mantenimiento
  • La STL se suele probar hasta la saciedad, por lo que suele tener menos errores que tu código.

Dicho esto, el bucle anterior podría quedar reducido a:

Código C++:
Ver original
  1. if( strcmp( localidad, lista[i].ciudad ) == 0 || strcmp( localidad, lista[i].ciudad2 ) == 0 )
  2.   centinela = 1;
  3. else
  4.   centinela = 0;

Fíjate que hemos sustituido un for y un if por únicamente un if

Para chequeos SI/NO es preferible usar el tipo bool

En C++ tienes de forma nativa el tipo bool. Este tipo permite almacenar dos valores diferentes: true y false.

Este tipo viene a sustituir los usos típicos de C en los que se partía de una variable "int" que únicamente almacenaba 0 (false) o 1 (true). En tu caso, centinela debería ser bool:

Código C++:
Ver original
  1. bool centinela = false;
  2.  
  3. //comparar los campos para comprobar si hay coincidencias en alguna struct
  4. if( strcmp( localidad, lista[i].ciudad ) == 0 || strcmp( localidad, lista[i].ciudad2 ) == 0 )
  5.   centinela = true;
  6.            
  7. //si la comparación ha sido existosa, se muestra el dato
  8. if(centinela)

Por cierto, como habrás podido ver en este ejemplo, no es necesario declarar las variables al inicio del programa. También las puedes declarar entre medias del código.

Los números no se pueden convertir... a mayúsculas

Código C++:
Ver original
  1. cout<<"Aqui muestro la oficina que coincide con el criterio de busqueda"<<toupper(lista[i].numero)

Ese toupper ahí no puede ser bueno... y no es el único caso.

Cuidado con los rangos

Código C++:
Ver original
  1. for(int i=0;i<154;i++)
  2. {
  3.   if(lista[i].numero == num_of)

Este es uno de los problemas que aparecen por poner valores "a pelo" en el código.

Si lista es un arreglo de 100 elementos... ¿por qué el for llega hasta 153?

Divide el código en funciones

Hacer un programa completo en una sola función únicamente está indicado para programas cortos (10 líneas, 20 quizás). Conforme el programa se vuelve más completo y complejo es imprescindible dividir el código en funciones.

Cada función tiene que tener una utilidad muy concreta y la idea que se persigue con esto es:
  • Que el código quede estructurado. Una función aisla su código del resto del programa
  • Permite reutilizar código de una forma muy sencilla
  • Permite modificar el funcionamiento del programa con grán facilidad. Si, por ejemplo, tienes que buscar un dato en una lista y tienes diferentes algoritmos de búsqueda en funciones diferentes, puedes cambiar el motor de búsqueda utilizado simplemente llamando a una función u otra en vez de reescribir el algoritmo.

En tu caso, un código que repites mucho es pasar los textos a minúsculas. Podrías tener una función tal que:

Código C++:
Ver original
  1. void AMinusculas( char* cadena )
  2. {
  3.   for( int i=0; i < strlen(cadena); ++i )
  4.     cadena[i] = tolower( cadena[i] );
  5. }

Aunque, bueno, también podríamos hacer uso de la STL:

Código C++:
Ver original
  1. #include <algorithm>
  2.  
  3. void AMinusculas( char* cadena )
  4. {
  5.   std::transform( cadena, &cadena[strlen(cadena)], cadena, ::tolower);
  6. }

El resultado al final va a ser el mismo.

Usa la clase string

Como te comenté, su uso proporciona bastantes ventajas, entre ellas un código más legible:

Código C++:
Ver original
  1. #include <string>
  2.  
  3. struct oficina{
  4.     int numero;
  5.     long CP, tlf;
  6.     string nombre;
  7.     string ciudad;
  8.     string direccion;
  9.     string ciudad2;
  10.     string municipio;
  11. };
  12.  
  13. // ...
  14.  
  15. lista[0].numero = 210;
  16. lista[0].CP = 30008;
  17. lista[0].tlf = 968244316;
  18. lista[0].nombre = "Murcia"; // Para copiar un texto se puede usar el operador de asignación
  19. lista[0].direccion = "Plaza Circular, 6";
  20. lista[0].ciudad = "Murcia";
  21. lista[0].municipio = "MURCIA";
  22.  
  23. // ... pasar a minúsculas puede ser más sencillo (basa
  24. void AMinusculas( string& cadena )
  25. {
  26.   std::transform( cadena.begin(), cadena.end(), cadena.begin(), ::tolower );
  27. }
  28.  
  29. // comparar dos cadenas es bastante sencillo:
  30. //comparar los campos para comprobar si hay coincidencias en alguna struct
  31. if(  localidad == lista[i].ciudad || localidad == lista[i].ciudad2 )

Puede que haya alguna cosilla más, pero ya son pequeñas tonterías.

Un saludo.