To many results just need one

[Diana Magers]

Ok, I have got my program working but it is displaying to many results. It is displaying the answer behind of the word

Sum

. If I input 'M' then the answer is still displaying behind the word

Sum

. Same thing for 'S' (for subtract) and 'D' (divide). Here is my code:


//This program will add, subtract, multiply or divide depending on what the user inputs

#include <iostream>

using namespace std;

int main()
{
char operation = ' ';
int num1 = 0;
int num2 = 0;
int answer = 0;

cout <<"Enter A (add) or S (subtract) or M (multiply) or D (divide): ";
cin >> operation;
operation = toupper(operation);

if (operation != 'A' && operation != 'S' && operation != 'S' && operation != 'M' && operation != 'D')

{
cout <<"Error: Wrong letter entered" << endl;
return 0;
}

cout <<"Enter first number: ";
cin >> num1;
cout <<"Enter second number: ";
cin >> num2;

if (operation == 'A' || 'a')
{
answer = num1 + num2;
cout <<"Sum: " << answer << endl;;

}

else if (operation == 'S' || 's')
{
if (num1 > num2)
{
answer = num1 - num2;
}
else
answer = num2 - num1;

cout <<"Difference: " << answer << endl;
}


else if (operation == 'M' || 'm')
{
answer = num1 * num2;
cout <<"Product: " << answer << endl;
}

else if (operation == 'D' || 'd')
{
if (num1 > num2)
{
answer = num1 / num2;
}
else
answer = num2 / num1;

cout <<"Quotient: " << answer << endl;
}

system("pause");
return 0;
}//end of main function

would someone please help me debug this program to find the problem? Thank you, Diana

[vlad from moscow]

First of all you should include header <cctype>

#include <iostream>
#include <cctype> 

Then as you converted operation to upper case there is no any sense to compare if with lower letter. So iit is enough to use for example the following condition

if ( operation == 'A' )

Any of your conditions as, for example,

if (operation == 'M' || 'm')

is always true because it corresponds to condition

if ( ( operation == 'M' ) || ( 'm' != 0 ) )

that is the right subexpression of the expression is always true because any letter has internal code that is not equal to zero. You should write

if (operation == 'M' || operation == 'm' )

But it is more simply to use

if ( operation == 'M' )

because early you wrote already

operation = toupper(operation);

[Need4Sleep]

If might be easier to store the methods in functions can call them in main, or ogranize them better. e.g.

int multipy(int x, int y){return x*y}
cout << "Product: " << multiply(num1,num2) << endl;

.Using this method, and replacing the If statements with cases(

http://www.cplusplus.com/doc/tutorial/control/

near the bottom) you could narrow this program down in main a lot:

operation = toupper(operation) //forces the char to become capitalized, saves worry of user 
//typing lowercase.
switch(operation)
{
    case 'A':
        cout << Addition(num1,num2) << endl;
        break; //ends the case selection
    case 'S':
        cout << Subtraction(num1,num2) << endl;
        break;
    case 'M':
        cout << Mult(num1,num2) << endl;
        break;
    case 'D':
        cout << Divide(num1,num2) << endl;
        break;
    default: //if nothing is called
        cout << "INVALID INPUT!" << endl;
        break;
}

*you have a repeated statement in your if statement: && operation != 'S'

*your subtraction method has wrong math. If i want to subtract 8 from 5, i should get
-3, but your program gives me postive 3. Same goes with your divide function.

*never use system("pause"), its very bad programming practice. The line of code is very memory heavy and inefficient. Try using other methods explained in the stickied topic in this forum

[Diana Magers]

Thank you, both replies I received.

Need4Sleep, I was wondering the

int multipy(int x, int y){return x*y}
cout << "Product: " << multiply(num1,num2) << endl;

I am not sure I understand your way of thinking. Could you possibility explain it better? Please?

I am definitely a beginner. I don't think we have used that yet in my program. I know I need to define the Addition, Subtraction, Multiply and Divide. I am a little confused. Sorry.

Diana

[vlad from moscow]

It is better to show code then to speak about it. :) For example you could write

#include <iostream>
#include <cctype>
#include <cstdlib>

using namespace std;

int Add(  int x, int y ) { return ( x + y ); }
int Substract( int x, int y ) { return ( x - y ); }
int Multiply( int x, int y ) { return ( x * y ); }
int Divide( int x, int y ) { return ( x / y ); }

typedef int ( *pf )( int, int );

int main()
{
   cout <<"Enter A (add) or S (subtract) or M (multiply) or D (divide): ";

   char operation;
   cin >> operation;
   operation = toupper( operation );

   pf action;
   const char *msg;

   switch ( operation )
   {
      case 'A':
         action = Add;
         msg    = "Sum: "
         break;
      case 'S':
         action = Substract;
         msg    = "Difference: ";
         break;
      case 'M':
         action = Multiply;
         msg    = "Product: ";
         break;
      case 'D':
         action = Divide;
         msg    = "Quotient: ";
         break;
      default:
         cout <<"Error: Wrong letter entered" << endl;
         system( "pause" );
         return 0;
   }
         

   cout <<"Enter first number: ";

   int num1;
   cin >> num1;

   cout <<"Enter second number: ";

   int num2;
   cin >> num2;

   if ( num2 > num1 )  // swap values for division
   {
      int tmp = num1;
      num1   = num2;
      num2   = tmp;
   } 

   int answer = action( num1, num2 );

   cout << msg << answer << endl;


   system( "pause" );
   
   return 0;
}//end of main function 

[Diana Magers]

Thank you vlad from moscow. I understand it I think but if I may ask you what is

pf action;
   const char *msg;

? I have never run into that as of yet in my programming. Very interesting though.

[vlad from moscow]

pf is definition of a pointer to function that returns int and has two parameters of type int.

So action is a variable of type pf, that is, is a pointer to function that returns int and has two parameters of type int.

Then you are assigning to action addresses of your functions depending on selection of a user.

msg is a pointer which will keep addresses of string literals "Sum: " and so on.

[Need4Sleep]

vlad from moscow shows some very practical(but advanced) code. I think a simple calling function might be better to understand(if i saw that code when i first started awhile back i think my brain would explode). You know common functions and return's right?