Switch in simple calculator

[Ostrich]

I'm having trouble using switch in a simple calculator to use a certain operator based upon an imput and it won't compare.

  #include <iostream>

using namespace std;

void calculata ()
{
    int Operaor;
    float numberOne;
    float numberTwo;
    float result;
    cout << "This is a program that performs addition, subtraction, multiplication and division\n\n";
    cout << "Input an equation (e.g 1+2)\n\n";
    cin >> numberOne >> Operaor >> numberTwo;
    switch (Operaor)
    {
    case "+":
        result = numberOne + numberTwo;
        cout << numberOne << "+" << numberTwo << "is: " << result << endl;
        break;
    case "-":
        result = numberOne - numberTwo;
        cout << numberOne << "-" << numberTwo << "is: " << result << endl;;
        break;
    case "*":
        result = numberOne * numberTwo;
        cout << numberOne << "*" << numberTwo << "is: " << result << endl;
        break;
    case "/":
        result = numberOne / numberTwo;
        cout << numberOne << "/" << numberTwo << "is: " << result << endl;
        break;
    }
}

int main()
{
    calculata();
}

The log says:
||=== Build: Debug in fred (compiler: GNU GCC Compiler) ===|
C:\Users\Ken\Documents\C++\fred\main.cpp||In function 'void calculata()':|
C:\Users\Ken\Documents\C++\fred\main.cpp|16|error: invalid conversion from 'const char*' to 'int' [-fpermissive]|
C:\Users\Ken\Documents\C++\fred\main.cpp|16|error: conversion from pointer type 'const char (*)[2]' to arithmetic type 'int' in a constant-expression|
||=== Build failed: 2 error(s), 0 warning(s) (0 minute(s), 0 second(s)) ===|

[Uk Marine]

Try using single quotes instead of double like so:

case '+':
.
.
.
case '-':
.
.
.

Also always return 0 in your main().

EDIT: Also I just noticed your "Operaor" variable is of type int and you are storing a char in it. None of these cases would execute unless you put the ASCII value for the corresponding characters.


In total your calculator is poorly written, try something like this next time:

#include <iostream>

using namespace std;

void calculata()
{
	enum { ADD = 1, SUBTRACT, MULTIPLY, NUM_OF_OPERATIONS };

	char Operator = ' ';
	float numberOne = 0.0f;
	float numberTwo = 0.0f;
	float result = 0.0f;
	int Selector = 0;

	cin >> numberOne >> Operator >> numberTwo;

	if (Operator == '+')
		Selector = 1;
	else if (Operator == '-')
		Selector = 2;
	else
		Selector = 3;

	switch (Selector)
	{
	case ADD:
		result = numberOne + numberTwo;
		cout << numberOne << "+" << numberTwo << "is: " << result << endl;
		break;
	case SUBTRACT:
		result = numberOne - numberTwo;
		cout << numberOne << "-" << numberTwo << "is: " << result << endl;;
		break;
	case MULTIPLY:
		result = numberOne * numberTwo;
		cout << numberOne << "*" << numberTwo << "is: " << result << endl;
		break;
	}
}

int main()
{
	calculata();

	int iTemp = 0;
	cin >> iTemp;
	return 0;
}

[lastchance]

@Ostrich, fundamentally it's good code. Your error messages are telling you that you can't convert from a const char* (something enclosed in " ") to an integer (as required by a switch statement). As @UK Marine pointed out, change the double quotes to single quotes (' ') indicating a char and make this the type of Operator (char is a low-byte representation of an integer).

Maybe also consider rearranging the code where you see what is (nearly) the same line appearing four times.

It isn't necessary to return anything explicitly from main unless you plan to interact with the operating system; it will return 0 automatically.

#include <iostream>
using namespace std;

void calculata()
{
    char Operator;                     // char, not int (sorry - I've changed the spelling too!)
    double numberOne, numberTwo;       // double will give you more accuracy and extend range
    double result;
    cout << "This is a program that performs addition, subtraction, multiplication and division\n\n";
    cout << "Input an equation (e.g 1+2) using operators +, -, * or /\n\n";
    cin >> numberOne >> Operator >> numberTwo;
    switch (Operator)
    {
    case '+':
        result = numberOne + numberTwo;
        break;
    case '-':
        result = numberOne - numberTwo;
        break;
    case '*':
        result = numberOne * numberTwo;
        break;
    case '/':
        result = numberOne / numberTwo;
        break;
    }
    cout << numberOne << Operator << numberTwo << "=" << result << endl;     // avoid repetition
}

int main()
{
    calculata();
}

[Chris Longwood]

@Uk marine

In total your calculator is poorly written, try something like this next time

Damn what a savage :p

[48T7M4Gy]

Don't forget to trap division by zero :)

[Ostrich]

Thank you so much everyone. I've taken a bit of everyone's advice (sorry @Uk marine, I want to understand my own code) and my family is very impressed.

[Chris Longwood]

@Ostrich, good job man! :)

Sorry for my previous comment, just realized how pointless it was...

btw I think Uk Marine was trying to show you a more cleaner implementation of your code (I think). For example, this is optional, but I think it's good practice to always initialize your variables because when they're not initialized then their values are indeterminate and you cannot rely on them being anything as the standard does not define them. Even statically allocated variables should be initialized. So if you want, you could initialize your variables and avoid a potential headache in the future. There is no good reason to not initialize a variable, plenty of good reasons to do the opposite. And enumerations are very good and I highly encourage you to use them when you have different operations and events to choose from, it makes your code slightly more readable. Like I can look at uk marine's enum and immediately understand what the whole thing is doing without having to look at the cases.. just an example.

Nonetheless, your calculator looks good, good job man =)