Sistema bancario (cajero automático) [C++] [CMD]

[HelThunk]

Buenas a todos, hoy vengo a presentarles un programa que he hecho (estoy totalmente abierto a criticas, tanto constructivas como destructivas), me ha llevado bastante tiempo (8 - 9 h) y mucho esfuerzo, no espero para nada que valoréis esto, es más, quiero que me digáis todo lo que pensáis, quiero mejorar (estoy aprendiendo de forma autodidacta) y simplemente, me anotare vuestras aportaciones, para intentar mejorar en futuros proyectos. Gracias con antelación *:rolleyes:

*Si alguien se siente ofendido al ver que, una persona que no tiene apenas conocimientos sobre este tema se quiere adentrar, pido disculpas con antelación (eso si, tengo mucho interés, así que mi objetivo es aprender, y no voy a parar hasta conseguirlo..)

*Me gusta la programación, y por ende, estoy intentando aprender por mi cuenta, me he visto muchos vídeos, he leído mucha información, pero como podéis comprobar, aun me falta un laaaaargo camino, el cual pienso recorrer con muchas ganas (sinceramente estoy muy perdido, así que cualquier ayuda es bienvenida).

Si vas a probar el programa;
Usuario; helthunk
Contraseña = 1234

EL CÓDIGO ES MUY LARGO Y SUPERO EL LIMITE DE 10 MIL CARACTERES, COLOCO PASTEBIN;

[URL="http://pastebin.com/0zjD21Qg"]http://pastebin.com/0zjD21Qg[/URL]

Todo consejo es bien acogido.

[lareto]

Hola; te dejo algunos comentarios de tipo general, y que tienen que ver con la forma antes que con el contenido.

Código:

#include "stdafx.h"

stdafx.h es un header generado por Visual Studio. No es que vaya a estar mal, sólo que en esta primera línea estás diciendo que todo lo que viene debajo ha sido escrito en y para un producto de microsoft.

Lo mismo para int _tmain(int argc, _TCHAR* argv[])
las formas estándar de la función main() son dos:
int main()
y
int main(int argc, char** arv)
como tu programa no usa los parámetros, deberías poder usar int main(), a secas.

Código:

#include <Windows.h>

Creo que estás incluyendo este archivo sólo para usar Sleep(). En general se prefiere adscribir a variantes estándar antes que usar funciones exclusivas de un sistema u otro. Para el caso de Sleep(), hay formas estándar (y sólo por eso, mejores) de producir una demora controlada. [por ejemplo: http://en.cppreference.com/w/cpp/thread/sleep_for]

Creo que para estudiar el C++ es mejor usar sólo características del estándar, evitando librerías y dependencias propias del compilador que se use o del sistema operativo. Deberías poder quitar Sleep() y system("cls"), que son sólo de Windows.

Código:

using namespace std;

No es de lo más aceptado en código de la vida real. Vuelca los miles de nombres del namespace std en la unidad de compilación, que en sistemas más complejos trae problemas, sobre todo si se cree que andar escribiendo "using namespace loquesea;" es normal; y no, no lo es. Es mejor dejar los nombre calificados con sus respectivos especificadores. Por ejemplo:
std::cout << "hola";
Otra opción puede ser:
using std::cout;
//... ...
cout << "hola";
En mi opinión "using namespace std;" es propio de vagos.

Código:

int saldo = 5000;
bool esalir = false;
bool salir1 = false;
bool eexit = false;

Variables globales... puede interesarte leer http://bytes.com/topic/c/insights/73...obal-variables

Código:

void menu(){

Hay una regla general de estilo que recomienda que una entidad (en este caso, una función) esté dedicada a una sola cosa ("cohesive responsibility"). Aquí menu() lo está haciendo todo.

Otra regla recomienda la brevedad (lo bueno si breve...). Una forma de abreviar sería evitar el código repetido, que veo que lo has intentado con la función menu1(), pero que no usas (por lo que deberías quitarla), pero sin embargo era una buena idea.

Bueno, hasta aquí mis comentarios / críticas sobre los aspectos formales; lo que no he probado es si funciona. ¿Dónde está este cajero?

[HelThunk]

Link del pastebin renovado ; http://pastebin.com/0zjD21Qg

Iniciado por lareto

Hola; te dejo algunos comentarios de tipo general, y que tienen que ver con la forma antes que con el contenido.

Código:

#include "stdafx.h"

stdafx.h es un header generado por Visual Studio. No es que vaya a estar mal, sólo que en esta primera línea estás diciendo que todo lo que viene debajo ha sido escrito en y para un producto de microsoft.

Lo mismo para int _tmain(int argc, _TCHAR* argv[])
las formas estándar de la función main() son dos:
int main()
y
int main(int argc, char** arv)
como tu programa no usa los parámetros, deberías poder usar int main(), a secas.

Perfecto, me lo anoto (qué hace exactamente lo de; int argc, _TCHAR* argv[] y poner un tmain en vez de main?)

Iniciado por lareto

Código:

#include <Windows.h>

Creo que estás incluyendo este archivo sólo para usar Sleep(). En general se prefiere adscribir a variantes estándar antes que usar funciones exclusivas de un sistema u otro. Para el caso de Sleep(), hay formas estándar (y sólo por eso, mejores) de producir una demora controlada. [por ejemplo: [url]http://en.cppreference.com/w/cpp/thread/sleep_for][/url]

Creo que para estudiar el C++ es mejor usar sólo características del estándar, evitando librerías y dependencias propias del compilador que se use o del sistema operativo. Deberías poder quitar Sleep() y system("cls"), que son sólo de Windows.

¿Podrías ponerme un ejemplo? (para el sleep standar)

Iniciado por lareto

Código:

using namespace std;

No es de lo más aceptado en código de la vida real. Vuelca los miles de nombres del namespace std en la unidad de compilación, que en sistemas más complejos trae problemas, sobre todo si se cree que andar escribiendo "using namespace loquesea;" es normal; y no, no lo es. Es mejor dejar los nombre calificados con sus respectivos especificadores. Por ejemplo:
std::cout << "hola";
Otra opción puede ser:
using std::cout;
//... ...
cout << "hola";
En mi opinión "using namespace std;" es propio de vagos.

Perfecto, me lo anoto, a partir de ahora pondré siempre el std:: manualmente.

Iniciado por lareto

Código:

int saldo = 5000;
bool esalir = false;
bool salir1 = false;
bool eexit = false;

Variables globales... puede interesarte leer [url]http://bytes.com/topic/c/insights/737451-case-against-global-variables[/url]

Si no utiizo variables globales, tendré que integrar dentro del main las variables no?(mi nivel de ingles es pésimo)

Iniciado por lareto

Código:

void menu(){

Hay una regla general de estilo que recomienda que una entidad (en este caso, una función) esté dedicada a una sola cosa ("cohesive responsibility"). Aquí menu() lo está haciendo todo.

Otra regla recomienda la brevedad (lo bueno si breve...). Una forma de abreviar sería evitar el código repetido, que veo que lo has intentado con la función menu1(), pero que no usas (por lo que deberías quitarla), pero sin embargo era una buena idea.

Al principio intentaba dividir todo en funciones, pero el problema vino cuando en la función menu() intentaba invocar a la función menu1(), me daba errores, intente muchas cosas diferentes, como cambiar el orden de estos, crear un menu3() que fuese = a menu1(), utilizar el menu1(void), colocarlo en forma de entero en vez de void.. y nada, ningún método me funciono, así que decidí ponerlo todo dentro del submeno.

Iniciado por lareto

Bueno, hasta aquí mis comentarios / críticas sobre los aspectos formales; lo que no he probado es si funciona. ¿Dónde está este cajero?

jajajaja, muchísimas gracias por tu ayuda, de momento solo disponemos de cajeros por el norte de España, pronto nos expandiremos hacia el sur y luego aremos un salto de continente, esperemos que llege pronto a tu ciudad ^^.

[razpeitia]

Uuuuufff no se por donde empezar, aunque las observaciones que haga coinciden con lo que dicen arriba.

1. Tu programa solo funciona en windows, ya que se trata de un cajero, incluso en los bancos reales esto es aceptable. Porque le designan cierto tipo de hardware y software para todos los cajeros (al menos la mayoría del tiempo así es).

2. Tu código es relativamente largo y monolitico, solo tienes 3 funciones definidas de las cuales. 1 es el main y 2 tienen el prefijo menu que parece que hacen todo menos ser un menu. Ademas la función menú es bastante larga, posiblemente la puedas romper en funciones mas pequeñas.

3. Tienes valores hardcodeados, como opcion == 3, WTF!? es 3?, no seria mejor usar enums y que sea mas descriptivo? O string hardcodeados por todos lados, seria imposible de internacionalizar eso.

4. Tu estilo de indentación es ni siquiera es conciso, mucho menos elegante.

5. Manchas tu namespace importando todo lo de std.

6. Usas variables globales. Solo me falto encontrar un goto.

[HelThunk]

Iniciado por razpeitia

2. Tu código es relativamente largo y monolitico, solo tienes 3 funciones definidas de las cuales. 1 es el main y 2 tienen el prefijo menu que parece que hacen todo menos ser un menu. Ademas la función menú es bastante larga, posiblemente la puedas romper en funciones mas pequeñas.

Como dije anteriormente;
'Al principio intentaba dividir todo en funciones, pero el problema vino cuando en la función menu() intentaba invocar a la función menu1(), me daba errores, intente muchas cosas diferentes, como cambiar el orden de estos, crear un menu3() que fuese = a menu1(), utilizar el menu1(void), colocarlo en forma de entero en vez de void.. y nada, ningún método me funciono, así que decidí ponerlo todo dentro del submenu'

Iniciado por razpeitia

3. Tienes valores hardcodeados, como opcion == 3, WTF!? es 3?, no seria mejor usar enums y que sea mas descriptivo? O string hardcodeados por todos lados, seria imposible de internacionalizar eso.

No he entendido este apartado, ¿qué quieres decir?, ¿Y me podrías decir como se usa enum?(con un ejemplo sería perfecto, en google solo he encontrado resultados en ingles, y mi nivel de ingles es muy básico)

Iniciado por razpeitia

5. Manchas tu namespace importando todo lo de std.

¿Qué quieres decir que mancho el namespace?
(Ya me han dicho que es mucho mejor poner el std:: manualmente, así, en futuros programas complejos se evita crear incompatibilidades)

[HelThunk]

Iniciado por lareto

Código:

#include "stdafx.h"

stdafx.h es un header generado por Visual Studio. No es que vaya a estar mal, sólo que en esta primera línea estás diciendo que todo lo que viene debajo ha sido escrito en y para un producto de microsoft.

Verás he estado probando lo que me has dicho.. y tengo el siguiente problema;

Código C++:

#include <iostream>
 
int main(){
    std::cout << "Hello world" <<std::endl;
    std::cin.get();
    return 0;
}
 

Si pongo el programa sin #include "stdafx.h" me tira un error, y en la descripción de este me dice que;

Código C++:

#include "stdafx.h"
#include <iostream>
 
int main(){
    std::cout << "Hello world" <<std::endl;
    std::cin.get();
    return 0;
}
 

En cambio si lo pongo, el programa me compila perfectamente, ¿qué puedo hacer?

[lareto]

Antes de confundirte de más con el tema del archivo "stdafx.h", creo que conviene aclarar que se trata del archivo cabecera que genera Visual Studio para agilizar el proceso de compilación, usando el mecanismo de "precompiled headers", que no está nada mal.

Lo que te señalaba era que como es un archivo propio del Visual Studio, los que usamos otros compiladores no lo tenemos y no sabemos qué es lo que has escrito en él, por lo que el código que pegas ya contiene en su primera línea algo que ignoramos.

En algunos foros algo más urbanizados se suele adoptar como criterio que el código que se muestre no necesita ser el código real sobre el que se está trabajando (miles de líneas, por ejemplo), sino una expresión mínima compilable, que sirva como ejemplo para expresar la duda; no debería faltar nada, pero tampoco conviene que sobre. En este sentido, poner #include "stdafx.h" o #include "otro_archivo_que_yo_sólo_me_sé.h" aporta la misma calidad de información.

Te sugiero que lo conserves para tus proyectos, y que aprendas a usarlo, pero si no aporta nada a la cuestión, no es necesario que lo incluyas en una pregunta o respuesta al foro.

Dicho esto, si quieres que no se genere, podrás inhibir la creación de esos "precompiled headers" al crear un nuevo proyecto "Win32 Console application" -> En el cuadro de Application Settings se puede destildar la casilla "Precompiled header". Si el proyecto ya está creado, puede deshabilitarse con: clic derecho sobre el proyecto en el Solution Explorer -> Properties -> Configuration Properties -> C/C++ -> Precompiled Headers -> Precompiled Header -> seleccionar "Not Using Precompiled Headers".

[HelThunk]

Muchas gracias, me ha quedado muy claro ^^

[HelThunk]

Iniciado por lareto

Código:

#include <Windows.h>

Creo que estás incluyendo este archivo sólo para usar Sleep(). En general se prefiere adscribir a variantes estándar antes que usar funciones exclusivas de un sistema u otro. Para el caso de Sleep(), hay formas estándar (y sólo por eso, mejores) de producir una demora controlada. [por ejemplo: [url]http://en.cppreference.com/w/cpp/thread/sleep_for][/url]

Creo que para estudiar el C++ es mejor usar sólo características del estándar, evitando librerías y dependencias propias del compilador que se use o del sistema operativo. Deberías poder quitar Sleep() y system("cls"), que son sólo de Windows.

¿Me puedes poner un ejemplo para usar un sleep standar?, he estado pensando y solo se me ocurre algo similar a esto..; (no entiendo la explicación de tu link);

Código C++:

#include <iostream>
int main(){
    int i;
    for (i = 10; i > -1; i--){
        int a = 100000000;
        while(a>-1){
            a--;
        }
        std::cout << i <<std::endl;
    }
 
    std::cin.get();
    return 0;
}
 

El problema que veo en el sleep que he creado manualmente, es que dependiendo del ordenador ira más rápido o más lento...

[lareto]

El código que pones produce la detención de la ejecución del programa (o del thread donde se ejecute), pero no sólo no puedes controlar su duración, sino que va a poner a uno de los núcleos del microprocesador al 100% (puedes comprobarlo con el "Administrador de tareas".

Te paso un ejemplo más simple donde se usa std::this_thread::sleep_for() para producir un "sleep". :
http://www.cplusplus.com/reference/t...ead/sleep_for/

Estos dos sitios son de referencia y consulta obligada:
http://en.cppreference.com/w/cpp
http://www.cplusplus.com/reference/

Creo que no debe haber programador de C++ que en este momento no tenga abierta una ventana con el navegador en alguno de estos dos. Como verás están en inglés. El material que se encuentra en castellano es en general más básico, que está muy bien para comenzar, o es de mala calidad. Supongo que no hay manera de hacerse buen programador en C++ escapándole al inglés.

[HelThunk]

Me voy a casar contigo!! jaja, ya lo he entendido a la perfección y/o recreado en el visual studio, he añadido a favoritos la pagina de cpplusplus, mi nivel de ingles es básico pero de momento me apaño, muchas gracias :D

[vangodp]

Creo que no debe haber programador de C++ que en este momento no tenga abierta una ventana con el navegador en alguno de estos dos.

Cierto! XDD