0

I've been learning C++, and I tried to create a basic calculator app. The goal is to obtain two numbers from 0-9 from the user, and a mathematical operation (+, -, *, /); if some other character is typed, I want to loop the program to keep prompting for the proper input.

But whenever I run the program, it doesn't recognize the numbers 0-9, and keeps repeating the loop. These are the main 3 functions I am using. From main, I'm simply calling them, so I doubt the problem is there. Help please?

Oh and I know I'm never supposed to use go-to, but I wanted practice. And if you could point out more efficient ways of writing this code, that's awesome. Thanks a million.

int GetUserInput(){
    using namespace std;
    cout << "Please enter a number between 0-9." << endl;
    
    char inputChar; 
    cin >> inputChar;

    while (inputChar != ('1' || '2' || '3' || '4' || '5' || '6' || '7' || '8' || '9' || '0')) {
        cout << "Please enter a number between 0-9." << endl;
        cin >> inputChar;
    }

    return static_cast <int> (inputChar);
}

char GetMathematicalOperation(){
    using namespace std;
    cout << "Please enter a mathematical operator (+, -, *, /)" << endl;

    // Storing user input character into char inputChar
    char inputChar; 
    
    inputloop:
    cin >> inputChar;
    switch(inputChar) {
        case('+'):
        case('-'):
        case('*'):
        case('/'):
            break;
        default:
            cout << "Please enter a mathematical operator (+, -, *, /)" << endl;
            goto inputloop;
        }

    return inputChar;
}

int CalculateResult(int x, char Operator, int y){
    if (Operator = '+')
        return x+y;
    if (Operator = '-')
        return x-y;
    if (Operator = '*')
        return x*y;
    if (Operator = '/')
        return x/y;
    return 0;
}
phuclv
  • 37,963
  • 15
  • 156
  • 475
  • Welcome to stackoverflow. After you fixed your first problem, please have a look at: http://www.parashift.com/c++-faq/stream-input-failure.html, then rewrite your input to `while(std::cin >> input)`. Also your `static_cast(input)` is likely not what you want. – Micha Wiedenmann Jul 18 '13 at 20:57
  • Please have a look here http://stackoverflow.com/questions/514420/how-to-validate-numeric-input-c – Razvan D Jul 18 '13 at 20:59
  • 1
    Use [`std::isdigit`](http://en.cppreference.com/w/cpp/string/byte/isdigit). – syam Jul 18 '13 at 21:05

7 Answers7

2

In C++

('1' || '2' || '3' || '4' || '5' || '6' || '7' || '8' || '9' || '0') == true

More specifically, a char that has a value that is not specifically 0 (the value, not the character) evaluates to true when compared with the == or != operator.

So your expression

inputChar != ('1' || '2' || '3' || '4' || '5' || '6' || '7' || '8' || '9' || '0')

Is equivalent to

inputChar != true

You would do better to put all those chars into a container and check if the user input exists in the container.

Untested code

char mychars[] = {'1','2','3','4','5','6','7','8','9','0'};
std::set<char> inputChars;
inputChars.insert(mychars, mychars+10);

if(inputChars.find(inputChar) != inputChars.end())
{
...
}
Cory Klein
  • 51,188
  • 43
  • 183
  • 243
  • "More specifically, a char that has a value that is not specifically 0 (the value, not the character) evaluates to true when compared with the == or != operator" ~ what does that mean? I don't understand, could you please rephrase? Also, what is a container? – NewtoProgramming Jul 18 '13 at 21:03
  • I added an example on how to use a container. As for using the `==` and `!=` operators, those operators will cast the arguments to their left and right into `bools`. If you cast a `char` into a `bool`, it will usually evaluate to `true`. – Cory Klein Jul 18 '13 at 21:07
2

The || operator needs to operate on boolean expressions, which characters are not. You'd need to expand it out to while (inputChar != '1' && inputChar != '2' && ....

Alternatively, you could exploit the fact that the character codes of digits are sequential. In other words, you could do while (inputChar < '0' || inputChar > '9').

Also, in your CalculateResult function, you need to change those = to == - otherwise, you overwrite the Operator variable, rather than comparing to it.

Drew McGowen
  • 11,471
  • 1
  • 31
  • 57
1

You could also use isdigit to do something like:

while(!isdigit(inputChar)) {  
    // code here  
}  
Rapptz
  • 20,807
  • 5
  • 72
  • 86
g3cko
  • 920
  • 1
  • 6
  • 21
1

Another solution: if (std::string("0123456789").find(inputChar) != std::string::npos). The variable npos - no position - means not found.

MSalters
  • 173,980
  • 10
  • 155
  • 350
0

You want to check whether the inputChar is outside the range '0' to '9', so you need something like this:

while (inputChar < '0' || inputChar > '9') 
James Holderness
  • 22,721
  • 2
  • 40
  • 52
  • People should in most cases rather use `while (std::cin >> input) ...` even though your answer is correct I think it is not didactic. – Micha Wiedenmann Jul 18 '13 at 20:58
  • @MichaWiedenmann I'm curious how you would do that and check that the input is in the desired range inside that one condition? – James Holderness Jul 18 '13 at 21:01
  • I would rather use two checks, something along the lines of `while (std::cin >> input) { if (!isValid(input)) { continue; } /* ... */ }`. Do you disagree? – Micha Wiedenmann Jul 18 '13 at 21:04
  • @MichaWiedenmann I don't think there's anything wrong with what you are suggesting, but that wasn't the problem the OP was trying to solve. When someone has a specific question, I prefer to concentrate on answering that question - not performing a full code review. I realise others may disagree with that approach, but you can always provide your own answer if you think it's important. I'm not sure it warranted downvoting my answer, but that's your choice. – James Holderness Jul 18 '13 at 21:16
0
while (inputChar != ('1' || '2' || '3' || '4' || '5' || '6' || '7' || '8' || '9' || '0'))

You have to compare against each character.

while ((inputChar != '1') ||  (inputChar != '2') ....

Or simply -

while ((inputChar < 47) || (inputChar > 57))

Next,

if (Operator = '+')

Compiler should have given you warning. Its an assignment. You actually need == operator instead if you intend to do comparison.

Mahesh
  • 34,573
  • 20
  • 89
  • 115
  • Did you mean `while (inputChar > '9')`? Also, what if the user enters a space? That's less than `'9'`. – Drew McGowen Jul 18 '13 at 21:01
  • With out `''` them, it should work as there is an conversion between `char` to `int`. Also, ASCII value of space character is 32. [Demo](http://coliru.stacked-crooked.com/view?id=f0fbc54811062a295e488be2a7125080-f674c1a6d04c632b71a62362c0ccfc51). – Mahesh Jul 18 '13 at 21:06
  • Oh I can't believe I missed that, thanks a lot. Yeah I don't know why my compiler didn't throw an error, it has in the past for incorrect assignments. I'm using Visual Studio 2012, are there any known bugs with that? And why can't I compare my inputChar against '1' || '2' .... ? And how does inputChar > 9 work? Wouldn't inputChar have to be an int type to make such a comparison? – NewtoProgramming Jul 18 '13 at 21:06
  • Converting character '9' to `int` is value 57 (that's the ASCII code of '9'). – Drew McGowen Jul 18 '13 at 21:07
  • Mahesh what do you mean? I checked out the link, what conversion? – NewtoProgramming Jul 18 '13 at 21:10
0

Your condition is wrong...you need to check (inputchar != '0') && (inputchar != '1') && ... && (inputchar != '9')