First program: a simple calculator

I'm reading this very good book, C# Player's Guide, to learn C#. I then plan to make video games with Unity 3D after.

In one of the chapters, he asks us to make a (really) simple calculator as a console application using the switch statement:

The program that we’ll make is going to be a simple calculator. We’re going to ask the user to type in two numbers and then type in a math operation to perform on the two numbers.

Use a switch statement to handle the different operations in different ways. Allow the user to type in ’+’ for addition, ’-’ for subtraction, ’*’ for multiplication, ’/’ for division, and ’%’ for remainder.

What do you think of my first program? Is it well-formatted and well-written?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace SimpleCalculator
{
class Program
{
static void Main(string[] args)
{
//use for the string operation choice
int operation = 0;
//stock the result of the operation
double result = 0;

Console.WriteLine("Type you first number :");
double firstNumber = Convert.ToDouble(stringFirstNumber);

Console.WriteLine("Type you second number :");
double secondNumber = Convert.ToDouble(stringSecondNumber);

Console.WriteLine("Enter the operation + (addition), - (soustraction), * (multiplication), / (division), ^ (exposant) or % (reste) :");

// Convert string choice to integral
if (stringOperation == "+" || stringOperation == "addition")
{
operation = 1;
}
else if (stringOperation == "-" || stringOperation == "soustraction")
{
operation = 2;
}
else if (stringOperation == "*" || stringOperation == "multiplication")
{
operation = 3;
}
else if (stringOperation == "/" || stringOperation == "division")
{
operation = 4;
}
else if (stringOperation == "^" || stringOperation == "exposant")
{
operation = 5;
}
else if (stringOperation == "%" || stringOperation == "reste")
{
operation = 6;
}

//Do someting depending on the operation choose
switch (operation)
{
case 1:
result = firstNumber + secondNumber;
break;

case 2:
result = firstNumber - secondNumber;
break;

case 3:
result = firstNumber * secondNumber;
break;

case 4:
result = firstNumber / secondNumber;
break;

case 5:
result = Math.Pow(firstNumber, secondNumber);
break;

case 6:
result = firstNumber % secondNumber;
break;
}
Console.WriteLine("\nResult of " + firstNumber + " " + stringOperation + " " + secondNumber + " = " + result + ".");
}
}
}

• With respect to everyone that provided an answer, the first thing to do when writing C# code is "thinking in C#". Even implementing a simple calculator should involve some OOD/OOP (defining classes, properties, hiding some details, exposing events, etc.). – Delphi.Boy May 25 at 19:23

First of all you are not verifying the input of your program. You also don't need the strings stringFirstNumber stringSecondNumber you can just put this all into a method that will check if the input is in correct format and also return the value something like this :

    private static double SetNumber(string outputText)
{
double parse;
Console.Write(outputText);
while (!double.TryParse(tempInput, out parse))
{
Console.WriteLine("Incorrect input !");
Console.Write(outputText);
}
return double.Parse(tempInput);
}


and use it like this :

        double firstNumber = SetNumber("Type you first number : ");
double secondNumber = SetNumber("Type you second number: ");


Now, about the operations.. I'm not sure if you wanted the sign % to work as percentage because if that's the case it wont. This is called modulus you can read more about it here https://msdn.microsoft.com/en-us/library/0w4e0fzs.aspx if you want to have percentages you must implement this yourself. Anyway I would recommend creating an array of strings that holds all your currently available operations and store them there something like this :

private static readonly string[] operations = { "+", "-", "*", "/", "^", "%" };


Note that we are creating this variable outside of the static void Main(). Here we are seeing a few modifiers which might be new to you so let me quickly go over them.

• private means we are keeping this variable only in our current class (Program)

• static means we are having just 1 instance of this variable.

• readonly means we wont modify those variables it's similar to const but it gives us a little bit more flexibility of where we can declare the value of the variable.

If the modifiers confuse you just remove them :

static string[] operations = { "+", "-", "*", "/", "^", "%" };


You need the static one since we are using this variable in static methods.

Let's move on. Once we have our operations declared in a string array we can easily check if the user is entering a correct input by creating the following function :

    private static bool IsValidOperation(string input)
{
return operations.Contains(input);
}


Well we have the checker method which uses LINQ.Contains() this extension checks if a specific collection contains a specific value. Now we just need a method that will return a string value to our stringOperation

    private static string SetOperation(string outputText)
{
Console.Write(outputText);
while (!IsValidOperation(tempInput))
{
Console.WriteLine("Incorrect input !");
Console.Write(outputText);
}
return tempInput;
}


and use like this

string stringOperation =
SetOperation(
"Enter the operation + (addition), - (soustraction), * (multiplication), / (division), ^ (exposant) or % (reste) :");


Now notice that we are using only string's. I removed the integer idea that you had initially. You can still implement it pretty easily working around with the indexes of the string array, but I don't this is necessary. Also one thing to mention is that we are again taking a parameter in our method SetOperation well we could've avoided that since we have just 1 operation to be entered so we could've just do

Console.Write("Enter the operation + (addition), - (soustraction), * (multiplication), / (division), ^ (exposant) or % (reste) :");


Instead of Console.Write(outputText);. But I used the same parameters in the previous method too so it's good to keep some consistency if you are still learning. We are almost done now all that's left is the big mess with if/else if statements and a switch case too .. So as I said earlier using integer and string is useless so I went just for a string. Combining both the switch case and the if/else if statements into this :

        switch (stringOperation)
{
case "+":
result = firstNumber + secondNumber;
break;
case "-":
case "soustraction":
result = firstNumber - secondNumber;
break;
case "*":
case "multiplication":
result = firstNumber*secondNumber;
break;
case "/":
case "division":
result = firstNumber/secondNumber;
break;
case "^":
case "exposant":
result = Math.Pow(firstNumber, secondNumber);
break;
case "%":
case "reste":
result = firstNumber%secondNumber;
break;
}


I prefer the switch case version here because it's more readable but you can go with if statements too

        if (stringOperation == "+" || stringOperation == "addition")
{
result = firstNumber + secondNumber;
}
else if (stringOperation == "-" || stringOperation == "soustraction")
{
result = firstNumber - secondNumber;
}
else if (stringOperation == "*" || stringOperation == "multiplication")
{
result = firstNumber*secondNumber;
}
else if (stringOperation == "/" || stringOperation == "division")
{
result = firstNumber/secondNumber;
}
else if (stringOperation == "^" || stringOperation == "exposant")
{
result = Math.Pow(firstNumber, secondNumber);
}
else if (stringOperation == "%" || stringOperation == "reste")
{
result = firstNumber%secondNumber;
}


Notice that we don't have else/default at the end in case of wrong input, because we already made sure the nasty user wont be able to proceed unless he sticks to our rules.

And lastly I edited this line

Console.WriteLine("\nResult of " + firstNumber + " " + stringOperation + " " + secondNumber + " = " + result + ".");


into a formated string which also looks a lot more readable :

Console.WriteLine("Result of {0} {1} {2} = {3}", firstNumber, stringOperation, secondNumber, result);


Here's the full code if you got confused where to put something :

    private static readonly string[] operations = { "+", "-", "*", "/", "^", "%" };
static void Main(string[] args)
{
double result = 0;
double firstNumber = SetNumber("Type you first number : ");
double secondNumber = SetNumber("Type you second number: ");

string stringOperation =
SetOperation(
"Enter the operation + (addition), - (soustraction), * (multiplication), / (division), ^ (exposant) or % (reste) :");

switch (stringOperation)
{
case "+":
result = firstNumber + secondNumber;
break;
case "-":
case "soustraction":
result = firstNumber - secondNumber;
break;
case "*":
case "multiplication":
result = firstNumber*secondNumber;
break;
case "/":
case "division":
result = firstNumber/secondNumber;
break;
case "^":
case "exposant":
result = Math.Pow(firstNumber, secondNumber);
break;
case "%":
case "reste":
result = firstNumber%secondNumber;
break;
}
Console.WriteLine("Result of {0} {1} {2} = {3}", firstNumber, stringOperation, secondNumber, result);
}

private static double SetNumber(string outputText)
{
double parse;
Console.Write(outputText);
while (!double.TryParse(tempInput, out parse))
{
Console.WriteLine("Incorrect input !");
Console.Write(outputText);
}
return double.Parse(tempInput);
}

private static bool IsValidOperation(string input)
{
return operations.Contains(input);
}

private static string SetOperation(string outputText)
{
Console.Write(outputText);
while (!IsValidOperation(tempInput))
{
Console.WriteLine("Incorrect input !");
Console.Write(outputText);
}
return tempInput;
}

• Thx for the reply and help, I will read it with attention tonight, Cheers – Francis Jasmin Apr 19 '16 at 20:19
• if TryParse() succeeds why do you return double.Parse(tempInput); in the SetNumber() method ? – Heslacher Apr 20 '16 at 6:42
• Correct we could have returned 'parse' and avoid converting – Denis Apr 20 '16 at 7:03
• Francis, if you have any questions feel free to ask me. – Denis Apr 20 '16 at 18:07
• Hi Denis. When we check for correct user input. Why did you choose double.TryParse instead of double.Parse ? As i understand it, double.Parse can't be use with logic statement (like if and while). Well, why can't I tell the computer to use something like "While (we can't convert string to double (ak : !double.parse(string)))" do the following code. – Francis Jasmin Apr 29 '16 at 18:29

So I've done the following:

• use of an enum as opposed to the magic number constants used for operations.
• use of multiple methods, to wit, one to determine what operation was selected and another to calculate based on operation
• input validation - for the input and output numbers as well as for the operation.
• use of switch over if..else.
• minor spelling bits (may have been a language difference?)

The code:

using System;

namespace SimpleCalculator
{
internal static class Program
{
private enum Operation
{
Unknown,

Subtraction,

Multiplication,

Division,

Exponentiation,

Modulo
};

private static void Main()
{

string stringFirstNumber;
double firstNumber;

do
{
} while (!double.TryParse(stringFirstNumber, out firstNumber));

string stringSecondNumber;
double secondNumber;

do
{
} while (!double.TryParse(stringSecondNumber, out secondNumber));

string stringOperation;
Operation operation;

do
{
// Ask user operation to use
Console.WriteLine("Enter the operation + (addition), - (subtraction), * (multiplication), / (division), ^ (exponent) or % (modulo) :");

// use for the string operation choice
operation = ConvertStringChoiceToEnum(stringOperation);
} while (operation == Operation.Unknown);

// stock the result of the operation
var result = DoSomethingByOperation(operation, firstNumber, secondNumber);

Console.WriteLine();
Console.WriteLine("Result of " + firstNumber + " " + stringOperation + " " + secondNumber + " = " + result + ".");
}

private static Operation ConvertStringChoiceToEnum(string stringOperation)
{
// Convert string choice to enum
switch (stringOperation)
{
case "+":
case "-":
case "subtraction":
return Operation.Subtraction;
case "*":
case "multiplication":
return Operation.Multiplication;
case "/":
case "division":
return Operation.Division;
case "^":
case "exponent":
return Operation.Exponentiation;
case "%":
case "modulo":
return Operation.Modulo;
default:
return Operation.Unknown;
}
}

private static double DoSomethingByOperation(Operation operation, double firstNumber, double secondNumber)
{
// Do something depending on the operation choose
switch (operation)
{
return firstNumber + secondNumber;

case Operation.Subtraction:
return firstNumber - secondNumber;

case Operation.Multiplication:
return firstNumber * secondNumber;

case Operation.Division:
return firstNumber / secondNumber;

case Operation.Exponentiation:
return Math.Pow(firstNumber, secondNumber);

case Operation.Modulo:
return firstNumber % secondNumber;

case Operation.Unknown:
return double.NaN;

default:
return double.NaN;
}
}
}
}

• Thx for the reply and help, I will read it with attention tonight, Cheers – Francis Jasmin Apr 19 '16 at 20:19
• Why the downvote, downvoter? – Jesse C. Slicer May 1 '16 at 15:29