7
\$\begingroup\$

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")
        {
            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 + ".");
        Console.ReadKey();
    }
  }
}
\$\endgroup\$
6
\$\begingroup\$

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;
                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 "+":
            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;
                break;
        }
        Console.WriteLine("Result of {0} {1} {2} = {3}", firstNumber, stringOperation, secondNumber, result);
        Console.ReadKey();
    }

    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);
    }

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

    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;
    }
\$\endgroup\$
  • 1
    \$\begingroup\$ Thx for the reply and help, I will read it with attention tonight, Cheers \$\endgroup\$ – Francis Jasmin Apr 19 '16 at 20:19
  • 3
    \$\begingroup\$ if TryParse() succeeds why do you return double.Parse(tempInput); in the SetNumber() method ? \$\endgroup\$ – Heslacher Apr 20 '16 at 6:42
  • \$\begingroup\$ Correct we could have returned 'parse' and avoid converting \$\endgroup\$ – Denis Apr 20 '16 at 7:03
  • \$\begingroup\$ Francis, if you have any questions feel free to ask me. \$\endgroup\$ – Denis Apr 20 '16 at 18:07
  • \$\begingroup\$ 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. \$\endgroup\$ – Francis Jasmin Apr 29 '16 at 18:29
2
\$\begingroup\$

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,

            Addition,

            Subtraction,

            Multiplication,

            Division,

            Exponentiation,

            Modulo
        };

        private static void Main()
        {

            // Ask user first number
            string stringFirstNumber;
            double firstNumber;

            do
            {
                Console.WriteLine("Type your first number :");
                stringFirstNumber = Console.ReadLine();
            } while (!double.TryParse(stringFirstNumber, out firstNumber));

            // Ask user second number
            string stringSecondNumber;
            double secondNumber;

            do
            {
                Console.WriteLine("Type your second number :");
                stringSecondNumber = Console.ReadLine();
            } 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) :");
                stringOperation = Console.ReadLine();

                // 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 + ".");
            Console.ReadKey();
        }

        private static Operation ConvertStringChoiceToEnum(string stringOperation)
        {
            // Convert string choice to enum
            switch (stringOperation)
            {
                case "+":
                case "addition":
                    return Operation.Addition;
                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)
            {
                case Operation.Addition:
                    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;
            }
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Thx for the reply and help, I will read it with attention tonight, Cheers \$\endgroup\$ – Francis Jasmin Apr 19 '16 at 20:19
  • \$\begingroup\$ Why the downvote, downvoter? \$\endgroup\$ – Jesse C. Slicer May 1 '16 at 15:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.