1
\$\begingroup\$

I created a simple calculator with multiplication, division, addition, and subtraction. Is there a way to write it shorter? I am a beginner so I do not know much about programming other than if...else statements and different data types.

        Console.WriteLine("Welcome to the simple calculator! The four operators of this calculator are +, -, *(multiply), and /(divide)");
        Console.Write("Number: ");
        string number = Console.ReadLine();
        Console.Write("Second Number: ");
        string secondNumber = Console.ReadLine();
        Console.Write("Operator: ");
        string operatorType = Console.ReadLine();

        var numberConversion = Convert.ToInt64(number);
        var secondNumberConversion = Convert.ToInt64(secondNumber);
        var sum = numberConversion + secondNumberConversion; //Result of addition
        var difference = numberConversion - secondNumberConversion; //Result of subtraction
        var product = numberConversion * secondNumberConversion; //Result of multiplication
        var quotient = numberConversion / secondNumberConversion; //Result of division

        if (operatorType == "+")
        {
            Console.WriteLine(sum);
        } else if (operatorType == "-")
        {
            Console.WriteLine(difference);
        } else if (operatorType == "*")
        {
            Console.WriteLine(product);
        } else if (operatorType == "/")
        {
            Console.WriteLine(quotient);
        } else
        {
            Console.WriteLine("Please enter the correct operator");
        }
\$\endgroup\$
5
  • \$\begingroup\$ I suggest moving the calculation inside the if/elseif tree, otherwise you may find that 2 + 0 causes a divide-by-zero error. \$\endgroup\$
    – Ben Voigt
    Aug 26, 2019 at 16:33
  • \$\begingroup\$ Or code golf :) \$\endgroup\$
    – ps2goat
    Aug 26, 2019 at 16:35
  • \$\begingroup\$ instead of convert.toint, use the tryparse method and handle exceptions if you have learned about them yet or getting to that lesson at some point. You want to be able to handle an error prone user typing in + + +.. \$\endgroup\$
    – terribletp
    Aug 26, 2019 at 16:42
  • \$\begingroup\$ Nice question. However, you should include your whole program (including the static void Main() method signature). \$\endgroup\$
    – user34073
    Aug 26, 2019 at 17:27
  • \$\begingroup\$ You need to understand that quality has usually nothing to do with how long your code is but how extensible and stable. But, of course, beginners tend to do many things in a roundabout and overly wordy way.. \$\endgroup\$
    – TaW
    Aug 26, 2019 at 17:28

2 Answers 2

1
\$\begingroup\$
  1. You need to check for input validity and have a mechanism for retry. Users can enter all sorts of things
  2. Your variable names aren't consistent. number and secondNumber. I would go with firstNumber and secondNumber in your scheme, but I would not use your scheme.
  3. Your variable names are too verbose: secondNumberConversion. How about n1 and n2?
  4. A simple, elegant, maintainable and extensible solution is to use a Dictionary.

Not addressing 1. here is my version:

Console.WriteLine("Welcome to the simple calculator!"
                  + " The four operators of this calculator are "
                  + "+, -, *(multiply), and /(divide)");

Console.Write("First Number: ");
Int64 n1 = Int64.Parse(Console.ReadLine());

Console.Write("Second Number: ");
Int64 n2 = Int64.Parse(Console.ReadLine());

Console.Write("Operator: ");
string operatorType = Console.ReadLine();

var operatorsFuncMap = new Dictionary<string, Func<Int64, Int64, Int64>> 
{
    {"+", (a, b) => a + b},
    {"-", (a, b) => a - b},
    {"*", (a, b) => a * b},
    {"/", (a, b) => a / b},
};

Console.WriteLine(operatorsFuncMap[operatorType](n1, n2));
\$\endgroup\$
0
\$\begingroup\$

Here's how I would do it:

        Console.WriteLine("Welcome to the simple calculator! The four operators of this calculator are +, -, *(multiply), and /(divide)");
        Console.Write("Number: ");
        string number = Console.ReadLine();
        Console.Write("Second Number: ");
        string secondNumber = Console.ReadLine();
        Console.Write("Operator: ");
        string operatorType = Console.ReadLine();

        var numberConversion = Convert.ToInt64(number);
        var secondNumberConversion = Convert.ToInt64(secondNumber);

        switch (operatorType)
        {
            case "+":
                Console.WriteLine(numberConversion + secondNumberConversion);
                break;
            case "-":
                Console.WriteLine(numberConversion - secondNumberConversion);
                break;
            case "*":
                Console.WriteLine(numberConversion * secondNumberConversion);
                break;
            case "/":
                Console.WriteLine(numberConversion / secondNumberConversion);
                break;
            default:
                Console.WriteLine("Please enter the correct operator");
                break;
        }

More info here on switch: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/switch

I prefer using switch over a long If Then just because it's easier to read.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.