HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

First program: a simple calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplefirstprogramcalculator

Problem

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;
using System.Threading.Tasks;

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;

//Ask user first number
Console.WriteLine("Type you first number :");
string stringFirstNumber = Console.ReadLine();
double firstNumber = Convert.ToDouble(stringFirstNumber);

//Ask user second number
Console.WriteLine("Type you second number :");
string stringSecondNumber = Console.ReadLine();
double secondNumber = Convert.ToDouble(stringSecondNumber);

//Ask user operation to use
Console.WriteLine("Enter the operation + (addition), - (soustraction), * (multiplication), / (division), ^ (exposant) or % (reste) :");
string stringOperation = Console.ReadLine();

// Convert string choice to integral
if (stringOperation == "+" || stringOperation == "addition")
{
operation = 1;
}
else if (stringOperation == "-" || stringOperation == "soustraction")
{

Solution

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);
        string tempInput = Console.ReadLine();
        while (!double.TryParse(tempInput, out parse))
        {
            Console.WriteLine("Incorrect input !");
            Console.Write(outputText);
            tempInput = Console.ReadLine();
        }
        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);
        string tempInput = Console.ReadLine();
        while (!IsValidOperation(tempInput))
        {
            Console.WriteLine("Incorrect input !");
            Console.Write(outputText);
            tempInput = Console.ReadLine();
        }
        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 "+":
case "addition":
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;

Code Snippets

private static double SetNumber(string outputText)
    {
        double parse;
        Console.Write(outputText);
        string tempInput = Console.ReadLine();
        while (!double.TryParse(tempInput, out parse))
        {
            Console.WriteLine("Incorrect input !");
            Console.Write(outputText);
            tempInput = Console.ReadLine();
        }
        return double.Parse(tempInput);
    }
double firstNumber = SetNumber("Type you first number : ");
        double secondNumber = SetNumber("Type you second number: ");
private static readonly string[] operations = { "+", "-", "*", "/", "^", "%" };
static string[] operations = { "+", "-", "*", "/", "^", "%" };
private static bool IsValidOperation(string input)
    {
        return operations.Contains(input);
    }

Context

StackExchange Code Review Q#126141, answer score: 6

Revisions (0)

No revisions yet.