C++

Forum

 
Shortening the Code

bleaz (38)
I have never coded anything before 2 weeks ago. i am trying to get as much knowledge as I can so I made this program to learn the basics so eventually I can get to the more complex codes that I could do for a living. I want your advice on my code on how I can shorten it or more effective ways to do certain things.

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
 
#include<iostream>
#include<math.h>
using namespace std;

int main(void)
{
    char rerun, rerun2;
    int pick;
    
    double dnumber1 = 0.0, dnumber2 = 0.0, dnumber3 = 0.0, daverage = 0.0;

    float price, slstax, total;
    double taxut = 0.06;
    
    double a1, a2, a3, d, r;
    int an;

    do {

       cout << "What Program do you want to run?" << endl << endl;
       cout << "1 for Average" << endl;
       cout << "2 for Tax" << endl;
       cout << "3 for the n-th term in a sequence." << endl;
       cin >> pick;
        
        if (pick == 1)
        {
         do {   
           cout << "Please enter 3 numbers for average: " << endl;
           cin >> dnumber1;
           cin >> dnumber2;
           cin >> dnumber3;
    
           daverage = (dnumber1 + dnumber2 + dnumber3) / 3;
    
           cout << "The average of the three numbers is " << daverage << endl << endl;
     
           cout << "Do you want to run that again (y/n)?" << endl;
           cin >> rerun;
           }
           while (rerun == 'y' || rerun == 'Y');
    
         }
    
         else if (pick == 2)
         {
          do {
             cout << "What does your item cost?" << endl;
             cin >> price;
             
             slstax = price * taxut;
             total = price + slstax;
             
             cout << "This is your final price with tax " << total << "." << endl << endl;
             
             cout << "Do you want to run that again (y/n)?" << endl;
             cin >> rerun;
             }
             while (rerun == 'y' || rerun == 'y');
         }
    
         else if (pick == 3)
         {
          do {
             cout << "Enter 3 numbers of the sequence." << endl;
             cin >> a1 >> a2 >> a3;
             
             cout << "Enter the term do you want to find?" << endl;
             cin >> an;
             
             if (a2 - a1 == a3 - a2)
             {
                    d = a2 - a1;
             
                    cout << "The "  << an << "th term is " << a1 + (an - 1) * d << "." << endl << endl;
                    }
                    
             else if (a2 / a1 == a3 / a2)
             {
                  r = a2 / a1;
                  
                  cout << "The " << an << "th term is " << pow (r,(an - 1)) * a1 << "." << endl << endl;
                  }
                  
             else if (a1 == a2)
             {
                  cout << "The " << an << "th term is " << a1 << "." << endl << endl;
                  }
             
             else 
             {
                  cout << "That sequence is neither Arithmatic or Geometric." << endl;
                  cout << "It cannot be computed." << endl << endl;
                  }
             
             cout << "Do you want to run that again (y/n)?" << endl << endl;
             cin >> rerun;
             }
             while (rerun == 'y' || rerun == 'y');
         }
    
         cout << "Do you want to pick another program (y/n)?" << endl;
         cout << "Picking 'n' will close program" << endl;
         cin >> rerun2;
         }
         while (rerun2 == 'y' || rerun2 == 'y');
    
}
theranga (125)
The only thing i can think of is reusing dnumber1, dnumber2, dnumber3 and daverage instead of a1,a2 and a3

- in general, reusing variables is a good idea because it saves a memory (which makes the program run faster)
cire (8284)
- in general, reusing variables is a good idea because it saves a memory (which makes the program run faster)


No on 3 counts.
Duthomhas (13119)
Use some functions. You could also use a switch statement there instead of a bunch of if..elses.

Try to keep variable declarations and their use as close together as possible.

Personally, I think that variable names should not be shortened any more than absolutely necessary. For example, 'slstax' is unnecessarily short for 'salestax' or 'sales_tax'.

Otherwise, it looks pretty good. You are way ahead of most people at your stage of programming newbishness.
TheIdeasMan (6781)
There are a few things I can see here:

Here is a better way of exiting out a loop (It was something I did to help someone else):

http://www.cplusplus.com/forum/beginner/84889/#msg455481


You have lots of these:

while (rerun2 == 'y' || rerun2 == 'y');

I think you meant to have one of those capitalised, but my code above shows a better way to do this.

I prefer not to use do loops unless I really have to - which is rare.

Next is code using doubles like this:

if (a2 - a1 == a3 - a2)

This will almost certainly evaluate to false. Consider this:

1
2
3
4
5
 
float a = 0.1;   // a == 0.09999997
float b = 10 * a; //  b == 0.9999997

if(a == b) // false


Changing the type to double doesn't help.

This is because floating point numbers (floats & doubles) are stored as binary fractions, and cannot represent every real number.

To fix this, you can have a variable which is the precision that you require:
double MyPrecision = 0.001

Then, if you want to check the equality of variable a & b, check that the absolute value of a- b is less than MyPrecision.

The next thing is to make use of functions. Printing the menu should be a function. Instead of having a lot of if else statements, you can use a switch statement. Each case of the switch should call a function that carries out that menu option.

Hope all goes well.
Last edited on
Duthomhas (13119)
Dang I'm tired. TheIdeasMan mentioned a few things I meant to but forgot -- including a FP issue I totally missed. Good thing he's on the ball!
bleaz (38)
To fix this, you can have a variable which is the precision that you require:
double MyPrecision = 0.001


Do I make a double with the name MyPrecision or do I just set all my doubles to 0.001?

Also I looked at the switch statements tutorial and I don't think I understand how to use them. Can I have an example replacing one of my if else with a switch?
Last edited on
TheIdeasMan (6781)
Do I make a double with the name MyPrecision or do I just set all my doubles to 0.001?


TheIdeasMan wrote:

To fix this, you can have a variable which is the precision that you require:
double MyPrecision = 0.001


Then, if you want to check the equality of variable a & b, check that the absolute value of a- b is less than MyPrecision.


Think carefully about what I said here. When you change my description into C++ code - the answer will be obvious.

Can I have an example replacing one of my if else with a switch?


The switch replaces a group of else if's. Each else if becomes a case inside the switch. Each case condition must be a constant integer or char, so you could only use it for the pick variable in your code.

As Duoas said more use of functions would be good. Displaying the menu and getting an input should be a function. Each case in the switch should call a function that carries out that menu item's action.

With variable names, my personal preference is to use CamelCase (Capitalise the first Letter of each word - no underscores) like this:

 
 
double SalesTax = 0.1;


I agree - you are doing well at this early stage of your learning - Hope all goes well for you, Iook forward to seeing your updated code.
bleaz (38)
Okay. I think I understand what you are saying. But I do not know where to put the absolute value part.

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
 
#include<iostream>
#include<math.h>
using namespace std;

int main(void)
{
    char Rerun, Rerun2;
    int Pick;

    double MyPrecision = 0.001
    double a1 = 0.001, a2 = 0.001, a3 = 0.001, d = 0.001, r = 0.001;

    int an;

    do 
          {
             cout << "Enter 3 numbers of the sequence." << endl;
             cin >> a1 >> a2 >> a3;
             
             cout << "Enter the term do you want to find?" << endl;
             cin >> an;
             
             if (a2 - a1 == a3 - a2) //would I put it here?
             {
                                               //or like here put abs (a1 - a2)> MyPrecision??
                    d = a2 - a1;
             
                    cout << "The "  << an << "th term is " << a1 + (an - 1) * d << "." << endl << endl;
             }
                    
             else if (a2 / a1 == a3 / a2)
             {
                  r = a2 / a1;
                  
                  cout << "The " << an << "th term is " << pow (r,(an - 1)) * a1 << "." << endl << endl;
             }
                  
             else if (a1 == a2)
             {
                  cout << "The " << an << "th term is " << a1 << "." << endl << endl;
             }
             
             else 
             {
                  cout << "That sequence is neither Arithmatic or Geometric." << endl;
                  cout << "It cannot be computed." << endl << endl;
             }
             
             cout << "Do you want to run that again (y/n)?" << endl;
             cin >> Rerun;
             }
             while (Rerun == 'y' || Rerun == 'Y');
          }


I think I get it I am just unclear about it.
bleaz (38)
Also I did what was said about a function I think... But it doesnt take out the if else statement or shorten the code. I don't usnderstand why it is needed or I should change it?
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
 
#include<iostream>
#include<math.h>
using namespace std;

void ProgramChoice () {
     cout << "What Program do you want to run?" << endl << endl;
     cout << "1 for Average" << endl;
     cout << "2 for Tax" << endl;
     cout << "3 for the n-th term in a sequence." << endl;
}

int main(void)
{
    char Rerun, Rerun2;
    int Pick;
    double MyPrecision = 0.001;
    
    double Average1 = 0.0, Average2 = 0.0, Average3 = 0.0, AverageAll = 0.0;

    float Price, SalesTax;
    double TaxUT = 0.06;
    
    double a1 = 0.001, a2 = 0.001, a3 = 0.001, d = 0.001, r = 0.001;
    int an;

    ProgramChoice ();
    cin >> Pick;
        
    if (Pick == 1) {
         do {   
           cout << "Please enter 3 numbers for average: " << endl;
           cin >> Average1;
           cin >> Average2;
           cin >> Average3;
    
           AverageAll = (Average1 + Average2 + Average3) / 3;
    
           cout << "The average of the three numbers is " << AverageAll << endl << endl;
     
           cout << "Do you want to run that again (y/n)?" << endl;
           cin >> Rerun;
           }
           while (Rerun == 'y' || Rerun == 'Y');
    
         }
    
         else if (Pick == 2)
         {
          do {
             cout << "What does your item cost?" << endl;
             cin >> Price;
             
             SalesTax = Price * TaxUT;
             
             cout << "This is your final price with tax " << Price + SalesTax << "." << endl << endl;
             
             cout << "Do you want to run that again (y/n)?" << endl;
             cin >> Rerun;
             }
             while (Rerun == 'y' || Rerun == 'Y');
         }
    
         else if (Pick == 3)
         {
          do 
          {
             cout << "Enter 3 numbers of the sequence." << endl;
             cin >> a1 >> a2 >> a3;
             
             cout << "Enter the term do you want to find?" << endl;
             cin >> an;
             
             if (a2 - a1 == a3 - a2)
             {
                    d = a2 - a1;
             
                    cout << "The "  << an << "th term is " << a1 + (an - 1) * d << "." << endl << endl;
             }
                    
             else if (a2 / a1 == a3 / a2)
             {
                  r = a2 / a1;
                  
                  cout << "The " << an << "th term is " << pow (r,(an - 1)) * a1 << "." << endl << endl;
             }
                  
             else if (a1 == a2)
             {
                  cout << "The " << an << "th term is " << a1 << "." << endl << endl;
             }
             
             else 
             {
                  cout << "That sequence is neither Arithmatic or Geometric." << endl;
                  cout << "It cannot be computed." << endl << endl;
             }
             
             cout << "Do you want to run that again (y/n)?" << endl;
             cin >> Rerun;
             }
             while (Rerun == 'y' || Rerun == 'Y');
          }
    
         cout << "Do you want to pick another program (y/n)?" << endl;
         cout << "Picking 'n' will close program" << endl;
         cin >> Rerun2;
}
TheIdeasMan (6781)
double a1 = 0.001, a2 = 0.001, a3 = 0.001, d = 0.001, r = 0.001;

This doesn't really do anything.

This is easy code to fix:

else if (a1 == a2)

Should look like this:

1
2
3
4
5
6
7
8
 
#include <cmath>  // for the abs function

const double Myprecsion = 0.001; //set this to whatever precision you want

else if (abs( a1 - a2 ) < Myprecsion ) {  //values are equal within Myprecsion (0.001)
//your code
}



Take this code as another example to fix:

if (a2 - a1 == a3 - a2)

Should look like this:

1
2
3
4
5
6
7
8
 
#include <cmath>  // for the abs function

const double Myprecsion = 0.001; //set this to whatever precision you want

if (abs( (a2 - a1) - (a3 - a2) )< Myprecsion ) {  //values are equal within Myprecsion (0.001)
//your code
}


Do something similar for the other FP comparisons you have.

Here is how to do a switch:

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
 
bool Quit = false;

while (!Quit) {

ProgramChoice; //could call it  ShowMenu

switch (Pick) {

     case 1:

           //call a function to do this menu option
           break;

     case  2:
           //call a function to do this menu option
           break;

     case  3:
           //call a function to do this menu option
           break;

     case  4:  //option to quit
           Quit = true;
           break;

     default:
           //code to deal with errors - invalid option
           break;
   } //end of switch
}//end of while
//execution continues from here if the quit option is selected


Rather than asking to rerun within each option, it is easier to show the whole menu again. This also saves having to test the value of the Rerun variable.

Hope all goes well.

Last edited on
bleaz (38)
The switch works but at the end it just goes wild. How do I get it to just close?

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
 
#include<iostream>
#include<math.h>
#include<cmath>
using namespace std;

void ShowMenu () {
     cout << "What Program do you want to run?" << endl << endl;
     cout << "1 for Average" << endl;
     cout << "2 for Tax" << endl;
     cout << "3 for the n-th term in a sequence." << endl;
}

int main(void)
{
    char Rerun, Rerun2;
    int Pick;
    const double MyPrecsion = 0.001;
    bool Quit = false;
    
    double Average1 = 0.0, Average2 = 0.0, Average3 = 0.0, AverageAll = 0.0;

    float Price, SalesTax;
    double TaxUT = 0.06;
    
    double a1, a2, a3, d, r;
    int an;

    while (!Quit) {

    ShowMenu ();
    cin >> Pick

    switch (Pick) {
         case 1:  
           cout << "Please enter 3 numbers for average: " << endl;
           cin >> Average1;
           cin >> Average2;
           cin >> Average3;
    
           AverageAll = (Average1 + Average2 + Average3) / 3;
    
           cout << "The average of the three numbers is " << AverageAll << endl << endl;

           break;
    
         case 2:
             cout << "What does your item cost?" << endl;
             cin >> Price;
             
             SalesTax = Price * TaxUT;
             
             cout << "This is your final price with tax " << Price + SalesTax << "." << endl << endl;
             
             break;
    
         case 3:
             cout << "Enter 3 numbers of the sequence." << endl;
             cin >> a1 >> a2 >> a3;
             
             cout << "Enter the term do you want to find?" << endl;
             cin >> an;
             
             if (a2 - a1 == a3 - a2)
             {
                    d = a2 - a1;
             
                    cout << "The "  << an << "th term is " << a1 + (an - 1) * d << "." << endl << endl;
             }
                    
             else if (a2 / a1 == a3 / a2)
             {
                  r = a2 / a1;
                  
                  cout << "The " << an << "th term is " << pow (r,(an - 1)) * a1 << "." << endl << endl;
             }
                  
             else if (a1 == a2)
             {
                  cout << "The " << an << "th term is " << a1 << "." << endl << endl;
             }
             
             else 
             {
                  cout << "That sequence is neither Arithmatic or Geometric." << endl;
                  cout << "It cannot be computed." << endl << endl;
             }
             
             break;
             
         case 4:
              Quit = true;
              break;
              
         default:
              cout << "Sorry invalid selection" << endl << endl;
              break;
         }
    }
}
Last edited on
TheIdeasMan (6781)
The switch works but at the end it just goes wild. How do I get it to just close?


What does it do exactly?


Update the showmenu function so it has the4th option to quit.


You still haven't fixed you double comparisons. And each case should call a function.
bleaz (38)
It repeats

cout << "What Program do you want to run?" << endl << endl;
cout << "1 for Average" << endl;
cout << "2 for Tax" << endl;
cout << "3 for the n-th term in a sequence." << endl;

over and over in the cmd prompt. (minus the code)

as well as when you type in someone other than quit or 123 it does the "what program..." as well.

I am still trying to wrap my head around the double comparisons fix you gave me.
Last edited on
TheIdeasMan (6781)
The code will keep repeating the menu until you enter 4 to quit. Any invalid input will repeat the menu as well.

It will make more sense if you update the showmenu function to show option 4 to quit..

It repeats ... over and over in the cmd prompt. (minus the code)


Does it stop to allow you to enter your menu selection?

I am still trying to wrap my head around the double comparisons fix you gave me.


You could copy the examples I gave into your code, then you only have to figure out the others - shouldn't be hard. What in particular do you not understand?
bleaz (38)
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
 
#include<iostream>
#include<math.h>
#include<cmath>
using namespace std;

int Pick;
const double MyPrecsion = 0.001;
bool Quit = false;

void ShowMenu () {
     cout << "What Program do you want to run?" << endl << endl;
     cout << "1 for Average" << endl;
     cout << "2 for Tax" << endl;
     cout << "3 for the n-th term in a sequence." << endl;
     cout << "4 to quit." << endl;
     cin >> Pick;
}

int main(void)
{
    double Average1 = 0.0, Average2 = 0.0, Average3 = 0.0, AverageAll = 0.0;

    float Price, SalesTax;
    double TaxUT = 0.06;
    
    double a1, a2, a3, d, r;
    int an;

    while (!Quit) {

    ShowMenu ();
        
    switch (Pick) {
         case 1:  
           cout << "Please enter 3 numbers for average: " << endl;
           cin >> Average1;
           cin >> Average2;
           cin >> Average3;
    
           AverageAll = (Average1 + Average2 + Average3) / 3;
    
           cout << "The average of the three numbers is " << AverageAll << endl;
           system("PAUSE");
           cout << endl;
           
           break;
    
         case 2:
             cout << "What does your item cost?" << endl;
             cin >> Price;
             
             SalesTax = Price * TaxUT;
             
             cout << "This is your final price with tax " << Price + SalesTax << "." << endl << endl;
             
             break;
    
         case 3:
             cout << "Enter 3 numbers of the sequence." << endl;
             cin >> a1 >> a2 >> a3;
             
             cout << "Enter the term do you want to find?" << endl;
             cin >> an;
             
             if (a2 - a1 == a3 - a2)
             {
                    d = a2 - a1;
             
                    cout << "The "  << an << "th term is " << a1 + (an - 1) * d << "." << endl << endl;
             }
                    
             else if (a2 / a1 == a3 / a2)
             {
                  r = a2 / a1;
                  
                  cout << "The " << an << "th term is " << pow (r,(an - 1)) * a1 << "." << endl << endl;
             }
                  
             else if (a1 == a2)
             {
                  cout << "The " << an << "th term is " << a1 << "." << endl << endl;
             }
             
             else 
             {
                  cout << "That sequence is neither Arithmatic or Geometric." << endl;
                  cout << "It cannot be computed." << endl << endl;
             }
             
             break;
             
         case 4:
              Quit = true;
              break;
              
         default:
              system("CLS");
              cout << "Sorry invalid selection" << endl << endl;
              break;
         }
    }
}


Okay whenever I type in anything but numbers it wiggs out and wont let me select anything.

Does it stop to allow you to enter your menu selection?


No. The last time I tried to run it the program itself wont close at all.
Topic archived. No new replies allowed.