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 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<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));
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<string, Func<int, int, int>>
{
{"x", (x, y) => x * y },
{"/", (x, y) => x / y },
{"+", (x, y) => x + y },
{"-", (x, y) => x - y }
};
var operation = prompter.Prompt<string>("Ok now enter your operation (" + string.Join(", ", operations.Keys) + ")", PromptOptions.Required, null, null, x => operations.Keys.Contains(x));
var firstNum = prompter.Prompt<int>("Enter the first number in your basic equation", PromptOptions.Required);
var secondNum = prompter.Prompt<int>("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? Simple. I want to add exponential calculation.
Simply add:
{"^", (x, y) => (int)Math.Pow(x, y)}
to our operations
dictionary. Now all our prompts and whatnot can support the power function.
This brings up a new issue: you have no support for decimal/double/float numbers. This is really easy to modify, simply replace all our int
types with double
.
Lastly, as for a bonus to this answer, you can also add alias operations. (What do I mean? Well what if I naturally use *
instead of x
for multiplication?)
This is super easy:
// Let's add our alias operations, */x
operations.Add("*", (x, y) => operations["x"](x, y));
Simply add them after the var operations
statement.
var operations = new Dictionary<string, Func<double, double, double>>
{
{"x", (x, y) => x * y },
{"/", (x, y) => x / y },
{"+", (x, y) => x + y },
{"-", (x, y) => x - y },
{"^", (x, y) => Math.Pow(x, y) },
{"%", (x, y) => x % y }
};
// Let's add our alias operations, */x
operations.Add("*", (x, y) => operations["x"](x, y));
Boom, now you can use *
instead of x
for multiplication. (But you can still use x
as well.)
Our entire programme is now down to 29 lines with appropriate whitespace, and supports much more than your original did. (Your original was 44 lines.)
var prompter = new ConsolePrompt(null);
Console.WriteLine("Hello, welcome to Alex's basic calculator!");
Console.ReadLine();
var operations = new Dictionary<string, Func<double, double, double>>
{
{"x", (x, y) => x * y },
{"/", (x, y) => x / y },
{"+", (x, y) => x + y },
{"-", (x, y) => x - y },
{"^", (x, y) => Math.Pow(x, y) },
{"%", (x, y) => x % y }
};
// Let's add our alias operations, */x
operations.Add("*", (x, y) => operations["x"](x, y));
do
{
var firstNum = prompter.Prompt<double>("Enter the first number in your basic equation", PromptOptions.Required);
var secondNum = prompter.Prompt<double>("Enter the second number in your basic equation", PromptOptions.Required);
var operation = prompter.Prompt<string>("Ok now enter your operation (" + string.Join(", ", operations.Keys) + ")", PromptOptions.Required, null, null, x => operations.Keys.Contains(x));
var answer = operations[operation](firstNum, secondNum);
Console.WriteLine($"{firstNum} {operation} {secondNum} = {answer}");
Console.ReadLine();
} while (prompter.Prompt("Enter exit to quit, anything else to continue", PromptOptions.Optional, "", parseResultMethod: x => x.ToLowerInvariant()) != "exit");
Pretty cool, eh?
All-in-all, very good start. Hopefully you learned a lot from this answer and can apply some of it to future applications. :)