How good is this code, speaking of good coding and optimization for a newbie?

Menu.cpp

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
#include<iostream>
#include"suma.h"
#include"resta.h"
#include"multi.h"
#include"mensaje.h"
#include"div.h"
#include"cuadricula.h"

using namespace std;

int main()

{
    int val1;
    int val2;
    int repe;
    int seleccion;

    mensaje miMensaje1;
    miMensaje1.mostrarMensaje();

    while( repe != 2 )

    {
        cout << "Escoge la opcion deseada \n(1)suma, (2)resta, (3)multiplicacion, (4)divicion, (5)cuadricula : ";

        cin  >> seleccion;

        switch( seleccion )

        {

            case 1:

            {
                cout << "Introduce el primer valor : ";
                cin  >> val1;

                cout << "\n\nIntroduce el segundo valor : ";
                cin  >> val2;

                cout << "\n\n";
                suma suma1( val1, val2 );
                cout << "El resultado de la suma es : " << suma1.obtenerResultado() << "!" << endl;
            }

            break;

            case 2:

            {
                cout << "Introduce el primer valor : ";
                cin  >> val1;

                cout << "\n\nIntroduce el segundo valor : ";
                cin  >> val2;

                cout << "\n\n";
                resta resta1( val1, val2 );
                cout << "\n\nEL resultado de la resta es : " << resta1.obtenerResultado() << "!" << endl;
            }

            break;

            case 3:

            {
                cout << "Introduce el primer valor : ";
                cin  >> val1;

                cout << "\n\nIntroduce el segundo valor : ";
                cin  >> val2;

                cout << "\n\n";
                multi multiple1( val1, val2 );
                cout << "\n\nEL resultado de la multiplicacion es : " << multiple1.obtenerResultado() << "!" << endl;
            }

            break;

            case 4:

            {
                cout << "Introduce el primer valor : ";
                cin  >> val1;

                cout << "\n\nIntroduce el segundo valor : ";
                cin  >> val2;

                cout << "\n\n";
                div divicion1( val1, val2 );
                cout << "\n\nEl resultado de la division es : " << divicion1.obtenerResultado() << "!" << endl;
            }

            break;

            case 5:

            {
                cuadricula miCuadricula;
                miCuadricula.mostrarCuadricula();
            }

            break;

        }

            cout << "\n\nDeseas seguir usando el programa? : ( 1 )SI  ( 2 )NO  :  ";
            cin  >> repe;

    }

    miMensaje1.mostrarDespedida();
}


suma.h

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
#ifndef SUMA_H_INCLUDED
#define SUMA_H_INCLUDED
#endif // SUMA_H_INCLUDED

#include<iostream>

using namespace std;

class suma

{
    public:

        explicit suma( int op1, int op2 )

        :opcion1( op1 ), opcion2( op2 ), resultado(op1 + op2)

        {
        }

        void establecerDatos( int op1, int op2 )

        {
            resultado = op1 + op2;
        }

        int obtenerResultado() const

        {
            return resultado;
        }


    private:

        int opcion1;
        int opcion2;
        int resultado;
};


multi.h

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
#ifndef MULTI_H_INCLUDED
#define MULTI_H_INCLUDED
#endif // MULTI_H_INCLUDED

#include<iostream>

using namespace std;

class multi

{
    public:

        explicit multi( int op1, int op2 )

        :opcion1( op1 ), opcion2( op2 ), resultado(op1 * op2)

        {
        }

        void establecerDatos( int op1, int op2 )

        {
            resultado = op1 * op2;
        }

        int obtenerResultado() const

        {
            return resultado;
        }

    private:

        int opcion1;
        int opcion2;
        int resultado;

};


mensaje.h

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
#ifndef MENSAJE_H_INCLUDED
#define MENSAJE_H_INCLUDED
#endif // MENSAJE_H_INCLUDED

#include<iostream>

using namespace std;

class mensaje

{
    public:

        explicit mensaje()

        {
        }

        void mostrarMensaje() const

        {
            cout << "Bienvenido a calculos v1.0 ! \n\n";
        }

        void mostrarDespedida() const

        {
            cout << "\n\nGracias por usar calculos v1.0 ! \n\n";
        }


    private:

        string texto;
};


div.h

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
#ifndef DIV_H_INCLUDED
#define DIV_H_INCLUDED
#endif // DIV_H_INCLUDED

#include<iostream>

using namespace std;

class div

{
    public:

        explicit div( double op1, double op2 )

        :opcion1( op1 ), opcion2( op2 ), resultado(op1 / op2)

        {
        }

        void establecerDatos( double op1, double op2 )

        {
            resultado = op1 / op2;
        }

        double obtenerResultado() const

        {
            return resultado;
        }


    private:

        double opcion1;
        double opcion2;
        double resultado;

};


cuadricula.h

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
#ifndef CUADRICULA_H_INCLUDED
#define CUADRICULA_H_INCLUDED
#endif // CUADRICULA_H_INCLUDED

#include<iostream>

using namespace std;

class cuadricula

{
    public:

        explicit cuadricula()

        {
        }

        void mostrarCuadricula() const

        {

            int x;
            int y;
            int z;

            cout << "*********************************************************\n";

            for( x = 0; x < 8; x++ )

            {
                for( z = 0; z < 3; z++ )

                {

                    for( y = 0; y < 9; y++ )

                    {
                        cout << simbolo << "      ";
                    }

                    cout << "\n";

                }

                cout << "*********************************************************\n";
            }
        }

    private:

        char simbolo = '*';
};


resta.h

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
#ifndef RESTA_H_INCLUDED
#define RESTA_H_INCLUDED
#endif // RESTA_H_INCLUDED

#include<iostream>

using namespace std;

class resta

{
    public:

        explicit resta( int op1, int op2 )

        :opcion1( op1 ), opcion2( op2 ), resultado(op1 - op2)

        {
        }

        void establecerDatos( int op1, int op2 )

        {
            resultado = op1 - op2;
        }

        int obtenerResultado() const

        {
            return resultado;
        }


    private:

        int opcion1;
        int opcion2;
        int resultado;
};
In no particular order:

using namespace std; is dangerous. Putting it in header files is very dangerous and will make you very unpopular.

1
2
3
4
  int val1;
    int val2;
    int repe;
    int seleccion;

Declaring your variables far away from their use is bad.

class mensaje contains unused member variable, and pretty much should not exist. Creating classes purely to hold functions that have no state is bad.

while( repe != 2 ) You're using repep without setting it to a value. It could be anything at this point. It could be 2 already.

1
2
     int opcion1;
        int opcion2;

These don't get used so why do they exist?

:opcion1( op1 ), opcion2( op2 )
These values get set, and then never used.

establecerDatos None of these functions ever get called. Why do they exist?

Your classes are generally bad. There's no reason for any of them to exist; you're just using them to execute simple functions. Just have functions.



Last edited on
Topic archived. No new replies allowed.