tags:

views:

151

answers:

8

I have a 3 file program, basically teaching myself c++. I have an issue. I made a switch to use the math function. I need and put it in a variable, but for some reason I get a zero as a result.

Also another issue, when I select 4 (divide) it crashes... Is there a reason?

Main file:

#include <iostream>
#include "math.h"
#include <string>

using namespace std;

int opersel;
int c;
int a;
int b;
string test;

int main(){

cout << "Welcome to Math-matrix v.34"<< endl;
cout << "Shall we begin?" <<endl;

//ASK USER IF THEY ARE READY TO BEGIN 

string answer;
cin >> answer;

if(answer == "yes" || answer == "YES" || answer == "Yes")
{

           cout << "excellent lets begin..." << endl;

           cout << "please select a operator..." << endl  << endl;


           cout << "(1) + " << endl;
           cout << "(2) - " << endl;
           cout << "(3) * " << endl;
           cout << "(4) / " << endl;

           cin >> opersel;

           switch(opersel){

                  case 1:
                  c = add(a,b);
                  break;
                  case 2:
                  c = sub(a,b);
                  break;
                  case 3:
                  c = multi(a,b);
                  break;
                  case 4:
                  c = divide(a,b);
                  break;
                  default:
                  cout << "error... retry" << endl;

                  }// end retry


           cout << "alright, how please select first digit?" << endl;

           cin >> a;

           cout << "excellent... and your second?" << endl;

           cin >> b;

           cout << c;

           cin >> test;

           }else if (answer == "no" || answer == "NO" || answer == "No"){


                 }//GAME ENDS








}// end of int main 

Here is my math.h file

#ifndef MATH_H
#define MATH_H

int add(int a, int b);


int sub(int a, int b);



int multi(int a, int b);


int divide(int a, int b);

#endif

Here is my math.cpp:

int add(int a, int b)
{

 return a + b;   

}

int sub(int a, int b)
{

 return a - b;   

}

int multi(int a, int b)
{

 return a * b;   

}

int divide(int a, int b)
{

 return a / b;   

}






}// end of int main 
+5  A: 

You're calling your functions with a and b before you get the data from the user. Try saving the math function that they selected when they enter it, and move your switch to after you have asked them for a and b.

#include <iostream>
#include "math.h"
#include <string>

using namespace std;

int opersel;
int c;
int a;
int b;
string test;

int main(){

cout << "Welcome to Math-matrix v.34"<< endl;
cout << "Shall we begin?" <<endl;

//ASK USER IF THEY ARE READY TO BEGIN 

string answer;
cin >> answer;

if(answer == "yes" || answer == "YES" || answer == "Yes")
{

           cout << "excellent lets begin..." << endl;

           cout << "please select a operator..." << endl  << endl;


           cout << "(1) + " << endl;
           cout << "(2) - " << endl;
           cout << "(3) * " << endl;
           cout << "(4) / " << endl;

           cin >> opersel;              

           cout << "alright, how please select first digit?" << endl;

           cin >> a;

           cout << "excellent... and your second?" << endl;

           cin >> b;

           switch(opersel){

                  case 1:
                  c = add(a,b);
                  break;
                  case 2:
                  c = sub(a,b);
                  break;
                  case 3:
                  c = multi(a,b);
                  break;
                  case 4:
                  c = divide(a,b);
                  break;
                  default:
                  cout << "error... retry" << endl;

           }// end retry

           cout << c;

           cin >> test;

           }else if (answer == "no" || answer == "NO" || answer == "No"){       
                 }//GAME ENDS   
}// end of int main 
Ben Burnett
Also, I suspect it's crashing on divide because you're in debug mode and b is initialized to zero (dividing by zero is a bad thing).
mos
mos is correct. Since you do not specify values for these int's they are initializing to zero. However, mos, this should not be unique to debug mode. Even in release numbers should initialize to 0.
Ben Burnett
This is the right answer. You failed to initialize a,b,c before use.
DeadMG
@Ben You're right. I didn't pay attention to the fact that it's a global variable.@TIMOTHY Don't count on your variables being initialized to zero, even when they're global. It's not a good habit to get into.
mos
A: 

You need to move these:

   cout << "alright, how please select first digit?" << endl;
   cin >> a;
   cout << "excellent... and your second?" << endl;
   cin >> b;

to before the switch. If you are thinking that a function like this:

int f( int x, int y ) {
   return x + y;
}

somehow returns the expression "x + y" which can be evaluated later, it doesn't. It returns the result of evaluating x + y at that point in the program. There are languages that can return an expression, but C++ isn't one of them.

anon
A: 

It looks like you are calling your math functions (inside the switch statement) before you populate your input variables a and b. Move the cin calls to above the switch and your problems should go away.

The crash is most likely caused when you call divide(a,b) because a and b are both 0. In that case you divide by zero and the system won't be happy about that.

Al Crowley
A: 

You're calling the math functions in your switch statement before you've read a and b from the user, so it's getting called with whatever happens to be in those variables at the time. You need to move the switch after reading a and b from the user, before outputting c

This might be going overboard, but rather than using a switch to accomplish that, you can use an array of function pointers:

typedef int (*math_function)(int, int);
math_function fns[] = {NULL, add, sub, multi, divide};

cin >> opersel;
if(opersel <= 4)
    cout << fns[opersel](a, b);
else
    cout << "You fail :(";
Michael Mrozek
Since you are storing actual function pointers, you don't have to dereference here. `fns[opersel](a, b);` works also, because of the way function pointers work.
Dennis Zickefoose
@Dennis Ah, thanks. I use them rarely enough that I botch the syntax; I'm amazed I got the typedef right
Michael Mrozek
Technically, you didn't botch the syntax. Function pointers are funny that way. Back in college, I turned in a project sprinkled with `(****you)(assignment);` just because I realized I could :P
Dennis Zickefoose
@Dennis Ah yes; I frightened my coworkers when I discovered that worked
Michael Mrozek
A: 

You are doing your calculations before you even read a and b into their variables. The cin >> statements come after your switch statements.

mjmarsh
A: 

This is a function call:

c = add(a,b);

It assigns to c the return value of calling add with parameters a and b. At the time you're calling this function, a and b have not been initialized. Put your switch after this line:

cin >> b;

and things should work as you expect.

Mike Burton
A: 

c = add(a,b); within your switch statement doesn't store the function, it calls the function, passing it a and b as arguments. This is the same for all your other cases. But at that point in the program, you haven't actually set a and b yet. They are uninitialized, so they can be anything. Including 0, which would cause problems when you divide. Edit: Actually, I just realized they are globals. So they should be initialized to zero for you, which would guarantee that the divide option will fail. If a and b were local to within main, they would be uninitialized, and you could make no assumptions as to what they initially hold.

It is possible to store an address to a pointer, but its a relatively advanced topic, and I would recommend you wait until you have a better grasp on the basics before getting into it. For now, what you're better off doing is getting all your input from the user, and then processing it all at once. In pseudo-code, your loop would look something like this:

Get Operator
If Operator < 0 or Operator > 4 Then "Error"
Get A
Get B
Switch Operator
    1: Result = Add(A, B)
    2: Result = Subtract(A, B)
    3: Result = Multiply(A, B)
    4: Result = Divide(A, B)
Output result

Note that there are a lot of "gotchas" when it comes to user input. For instance, when you ask the user to input the value for a, what if they enter "Five"? At the very least, you'll want to check the status of cin after any input; this will tell you if it got any valid data from the user or not. cin.good() should tell you if the previous operation was successful, and if not you can quit, or try again, or whatever you like.

Dennis Zickefoose
A: 

Here's an idea (using function objects or functors):

#include <iostream>
using std::cin;
using std::cout;
struct Math_Operation
{
  virtual int operator()(int a, int b) = 0;
};

struct Math_Add : Math_Operation
{
  int operator()(int a, int b)
  { return a + b;}
};

struct Math_Sub : Math_Operation
{
  int operator()(int a, int b)
  { return a - b;}
};

struct Math_Mul : Math_Operation
{
  int operator()(int a, int b)
  { return a * b;}
};

struct Math_Div : Math_Operation
{
  int operator()(int a, int b)
  { return a / b;}
};


int main(void)
{
  cout << "Enter operation (+, -, *, /): ";
  cout.flush();
  char operation;
  Math_Operation * p_math_opr = NULL;
  cin >> operation;
  switch (operation)
  {
    case '+':
      p_math_opr = new Math_Add;
      break;
    case '-':
      p_math_opr = new Math_Sub;
      break;
    case '*':
      p_math_opr = new Math_Mul;
      break;
    case '/':
      p_math_opr = new Math_Div;
      break;
    default:
      p_math_opr = NULL;
  }

// ...Input numbers into A & B...
  int A = 5;
  int B = 8;

//  Perform calculation with A & B
  int Result = 0;
  if (p_math_opr)
  {
      Result = (*p_math_opr)(A, B);
      delete p_math_opr;
  }
  cout << "Result is: " << Result << "\n";
  return 0;
}

The functor approach allows you to save an operation. Using pointers to base class allows you to execute the functor without knowing which operation is "in-use".

Edit:
1. Added int to function definitions in child classes.
2. Corrected execution syntax of functor.
3. Added deletion of functor.

Thomas Matthews
Leaks are baaaad.
Dennis Zickefoose
Also, this doesn't actually work as written, since you can't overload `()` for pointers. You want `Result = (*p_math_opr)(A, B);` or possibly `Result = p_math_opr->operator()(A, B);`
Dennis Zickefoose
And switch statements combined with inheritance are not so great, either. You could use a look-up table, which would also avoid `new`.
Steven Sudit
@Dennis Zikefoose: Thanks for code review, incorporated changes.
Thomas Matthews
@Steven Sudit: Switch statements and inheritance are called *Factories*. A look up table could be used, but the OP wanted to "put the function in a variable", thus the use of functors. If we want to better the OP's design, we would implement lexing and a grammar tree; perhaps even regular expressions.
Thomas Matthews
@Thomas: A table of function pointers would be the most direct approach to putting a function in a variable. Arguably, OP is leaning towards a simple expression evaluator that uses a stack to convert from infix to postfix and process the result.
Steven Sudit