Problem
I created a simple calculator with multiplication, division, addition, and subtraction. Is there a way to write it shorter? I am a beginner so I do not know much about programming other than if…else statements and different data types.
Console.WriteLine("Welcome to the simple calculator! The four operators of this calculator are +, -, *(multiply), and /(divide)");
Console.Write("Number: ");
string number = Console.ReadLine();
Console.Write("Second Number: ");
string secondNumber = Console.ReadLine();
Console.Write("Operator: ");
string operatorType = Console.ReadLine();
var numberConversion = Convert.ToInt64(number);
var secondNumberConversion = Convert.ToInt64(secondNumber);
var sum = numberConversion + secondNumberConversion; //Result of addition
var difference = numberConversion - secondNumberConversion; //Result of subtraction
var product = numberConversion * secondNumberConversion; //Result of multiplication
var quotient = numberConversion / secondNumberConversion; //Result of division
if (operatorType == "+")
{
Console.WriteLine(sum);
} else if (operatorType == "-")
{
Console.WriteLine(difference);
} else if (operatorType == "*")
{
Console.WriteLine(product);
} else if (operatorType == "/")
{
Console.WriteLine(quotient);
} else
{
Console.WriteLine("Please enter the correct operator");
}
Solution
- You need to check for input validity and have a mechanism for retry. Users can enter all sorts of things
- Your variable names aren’t consistent.
number
andsecondNumber
. I would go withfirstNumber
andsecondNumber
in your scheme, but I would not use your scheme. - Your variable names are too verbose:
secondNumberConversion
. How aboutn1
andn2
? - A simple, elegant, maintainable and extensible solution is to use a Dictionary.
Not addressing 1. here is my version:
Console.WriteLine("Welcome to the simple calculator!"
+ " The four operators of this calculator are "
+ "+, -, *(multiply), and /(divide)");
Console.Write("First Number: ");
Int64 n1 = Int64.Parse(Console.ReadLine());
Console.Write("Second Number: ");
Int64 n2 = Int64.Parse(Console.ReadLine());
Console.Write("Operator: ");
string operatorType = Console.ReadLine();
var operatorsFuncMap = new Dictionary<string, Func<Int64, Int64, Int64>>
{
{"+", (a, b) => a + b},
{"-", (a, b) => a - b},
{"*", (a, b) => a * b},
{"/", (a, b) => a / b},
};
Console.WriteLine(operatorsFuncMap[operatorType](n1, n2));
Here’s how I would do it:
Console.WriteLine("Welcome to the simple calculator! The four operators of this calculator are +, -, *(multiply), and /(divide)");
Console.Write("Number: ");
string number = Console.ReadLine();
Console.Write("Second Number: ");
string secondNumber = Console.ReadLine();
Console.Write("Operator: ");
string operatorType = Console.ReadLine();
var numberConversion = Convert.ToInt64(number);
var secondNumberConversion = Convert.ToInt64(secondNumber);
switch (operatorType)
{
case "+":
Console.WriteLine(numberConversion + secondNumberConversion);
break;
case "-":
Console.WriteLine(numberConversion - secondNumberConversion);
break;
case "*":
Console.WriteLine(numberConversion * secondNumberConversion);
break;
case "/":
Console.WriteLine(numberConversion / secondNumberConversion);
break;
default:
Console.WriteLine("Please enter the correct operator");
break;
}
More info here on switch
: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/switch
I prefer using switch
over a long If Then
just because it’s easier to read.