patterncsharpModerate
Simple calculator in C#
Viewed 0 times
simplecalculatorstackoverflow
Problem
It is a basic calculator where the user inputs two numbers, and an operation and the program makes it into an equation and gets the answer. For example, if the user enters number 2 number 3 and tells it to multiply, it will display the answer as 6. I just want to know how this could be improved.
Could I have used loops or more advanced procedures to get the same output?
```
namespace Test
{
class Program
{
static void Main(string[] args)
{
int firstNum;
int secondNum; //Variables for equation
string operation;
int answer;
Console.WriteLine("Hello, welcome to Alex's basic calculator!");
Console.ReadLine();
Console.Write("Enter the first number in your basic equation: ");
firstNum = Convert.ToInt32(Console.ReadLine());
//User input for equation
Console.Write("Now enter your second number in the basic equation: ");
secondNum = Convert.ToInt32(Console.ReadLine());
Console.Write("Ok now enter your operation ( x , / , +, -) ");
operation = Console.ReadLine();
if (operation == "x")
{
answer = firstNum * secondNum;
Console.WriteLine(firstNum + " x " + secondNum + " = " + answer);
Console.ReadLine();
}
else if (operation == "/")
{
answer = firstNum / secondNum;
Console.WriteLine(firstNum + " / " + secondNum + " = " + answer);
Console.ReadLine();
}
//Getting answers
else if (operation == "+")
{
answer = firstNum + secondNum;
Could I have used loops or more advanced procedures to get the same output?
```
namespace Test
{
class Program
{
static void Main(string[] args)
{
int firstNum;
int secondNum; //Variables for equation
string operation;
int answer;
Console.WriteLine("Hello, welcome to Alex's basic calculator!");
Console.ReadLine();
Console.Write("Enter the first number in your basic equation: ");
firstNum = Convert.ToInt32(Console.ReadLine());
//User input for equation
Console.Write("Now enter your second number in the basic equation: ");
secondNum = Convert.ToInt32(Console.ReadLine());
Console.Write("Ok now enter your operation ( x , / , +, -) ");
operation = Console.ReadLine();
if (operation == "x")
{
answer = firstNum * secondNum;
Console.WriteLine(firstNum + " x " + secondNum + " = " + answer);
Console.ReadLine();
}
else if (operation == "/")
{
answer = firstNum / secondNum;
Console.WriteLine(firstNum + " / " + secondNum + " = " + answer);
Console.ReadLine();
}
//Getting answers
else if (operation == "+")
{
answer = firstNum + secondNum;
Solution
I'm going to go over a few things in this answer, hopefully it makes them simple enough. Then, at the end, I'm going to go way overboard and significantly over-engineer this programme while still making it shorter.
First, let's talk about your input handling (or lack thereof). As programmers, we should strive to be able to gracefully handle any/all input, and do something appropriate as a result. For invalid input, usually we reprompt (sometimes we just gracefully fail), for valid input, we go to the next step.
You assume all input will be valid, which is a huge potential for problematic behavior. What happens if I enter
So, let's handle all that gracefully. I wrote a specific portion of a library I have written specifically for input handling, and gracefully doing so.
Basically, you need all three files from this GitHub folder. You will also need both the bottom files, from this other GitHub folder. You'll want to modify the namespaces to suit your structure. (You may also just download the entire repo, and build
Next, you'll want to create a
Once you've done that, you can simply use:
Now we've done three things here:
Next, we'll talk about your conditions. You use an
Instead, we'll go to a
By using a
Next, we'll talk about your output. You repeat certain statements excessively, let's find a better way to do that.
We have
Boom. We don't need the
But there's a better way to write this (with C#6.0, that is):
Using string interpolation we make it much easier to read what's going on.
You also had the idea for a
And now it will continue until the user types
So, we've improved everything you've got going, but there's still one more improvement we can make: convert all our operations to a dictionary.
What do I mean? Well I'll show you.
What did we just do? We just replaced the entire
First, let's talk about your input handling (or lack thereof). As programmers, we should strive to be able to gracefully handle any/all input, and do something appropriate as a result. For invalid input, usually we reprompt (sometimes we just gracefully fail), for valid input, we go to the next step.
You assume all input will be valid, which is a huge potential for problematic behavior. What happens if I enter
one for either number? Bad things, my friend, bad things.So, let's handle all that gracefully. I wrote a specific portion of a library I have written specifically for input handling, and gracefully doing so.
Basically, you need all three files from this GitHub folder. You will also need both the bottom files, from this other GitHub folder. You'll want to modify the namespaces to suit your structure. (You may also just download the entire repo, and build
Evbpc.Framework, which I recommend. All of this is beyond the scope of this answer.)Next, you'll want to create a
ConsolePrompt object:var prompter = new ConsolePrompt(null);Once you've done that, you can simply use:
var validOperations = new[] { "x", "/", "+", "-" };
var firstNum prompter.Prompt("Enter the first number in your basic equation", PromptOptions.Required);
var secondNum = prompter.Prompt("Now enter your second number in the basic equation", PromptOptions.Required);
var operation = prompter.Prompt("Ok now enter your operation (" + string.Join(", ", validOperations) + ")", PromptOptions.Required, validationMethod: x => validOperations.Contains(x));Now we've done three things here:
- We validate all the numbers and reprompt the user if the number is invalid.
- We make sure the user only entered a basic operation for the intended operation.
- We make a list of valid operations at the beginning, which means if we add one we can do so easily.
Next, we'll talk about your conditions. You use an
if statement for all your operations, but that's unnecessary and creates extra processing in the case that the operation is the last condition. (Each previous if is evaluated before it continues to the next.)Instead, we'll go to a
switch statement:switch (operation)
{
case "x":
answer = firstNum * secondNum;
break;
case "/":
answer = firstNum / secondNum;
break;
case "+":
answer = firstNum + secondNum;
break;
case "-":
answer = firstNum - secondNum;
break:
}By using a
switch we've made things substantially more clear: our intention is to only evaluate what operation is and do something based on it.Next, we'll talk about your output. You repeat certain statements excessively, let's find a better way to do that.
We have
firstNum and secondNum, as well as operation and answer. So how can we rewrite our Console.WriteLine methods to be more dynamic? Well, with the switch it's easy, right after the end brace for the switch we just write what each value was:Console.WriteLine(firstNum + " " + operation + " " + secondNum + " = " + answer);Boom. We don't need the
Console.WriteLine in each switch (or if) block. We just need one at the end.But there's a better way to write this (with C#6.0, that is):
Console.WriteLine($"{firstNum} {operation} {secondNum} = {answer}");Using string interpolation we make it much easier to read what's going on.
You also had the idea for a
while loop in your head to repeat the calculations, so let's do that:do
{
// All our calculator code
} while (prompter.Prompt("Enter exit to quit, anything else to continue", PromptOptions.Optional, "", parseResultMethod: x => x.ToLowerInvariant()) != "exit");And now it will continue until the user types
exit at that prompt.So, we've improved everything you've got going, but there's still one more improvement we can make: convert all our operations to a dictionary.
What do I mean? Well I'll show you.
var operations = new Dictionary>
{
{"x", (x, y) => x * y },
{"/", (x, y) => x / y },
{"+", (x, y) => x + y },
{"-", (x, y) => x - y }
};
var operation = prompter.Prompt("Ok now enter your operation (" + string.Join(", ", operations.Keys) + ")", PromptOptions.Required, null, null, x => operations.Keys.Contains(x));
var firstNum = prompter.Prompt("Enter the first number in your basic equation", PromptOptions.Required);
var secondNum = prompter.Prompt("Enter the second number in your basic equation", PromptOptions.Required);
var answer = operations[operation](firstNum, secondNum);What did we just do? We just replaced the entire
switch/if blocks, all the manually typing x, /, +, -, etc. with a dictionary of string -> function. Where does this benefit us? SimCode Snippets
var prompter = new ConsolePrompt(null);var validOperations = new[] { "x", "/", "+", "-" };
var firstNum prompter.Prompt<int>("Enter the first number in your basic equation", PromptOptions.Required);
var secondNum = prompter.Prompt<int>("Now enter your second number in the basic equation", PromptOptions.Required);
var operation = prompter.Prompt<string>("Ok now enter your operation (" + string.Join(", ", validOperations) + ")", PromptOptions.Required, validationMethod: x => validOperations.Contains(x));switch (operation)
{
case "x":
answer = firstNum * secondNum;
break;
case "/":
answer = firstNum / secondNum;
break;
case "+":
answer = firstNum + secondNum;
break;
case "-":
answer = firstNum - secondNum;
break:
}Console.WriteLine(firstNum + " " + operation + " " + secondNum + " = " + answer);Console.WriteLine($"{firstNum} {operation} {secondNum} = {answer}");Context
StackExchange Code Review Q#131158, answer score: 16
Revisions (0)
No revisions yet.