7
\$\begingroup\$

I am a beginner programmer, I decided to make this calculator program to test my knowledge on basic C# syntax, the code executes as intended, however I would like to shorten it to be more concise.

Console.WriteLine("Please select your first number: ");

double numberOne = Convert.ToDouble(Console.ReadLine());

Console.WriteLine("Please select your second number: ");

double numberTwo = Convert.ToDouble(Console.ReadLine());

Console.WriteLine("Please choose the operation: "); 

var userInput = Console.ReadLine(); 

if (userInput == "+") 
{
    Console.WriteLine(numberOne + numberTwo);
}
else if (userInput == "-")
{
    Console.WriteLine(numberOne - numberTwo);
}
else if (userInput == "*")
{
    Console.WriteLine(numberOne * numberTwo);
}
else if (userInput == "/")
{
    Console.WriteLine(numberOne / numberTwo); 
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Hi, please include the main class/function and imports so that people can just copy your code and compile it without having to add anything to it. \$\endgroup\$
    – slepic
    Commented Apr 24, 2023 at 5:17
  • 3
    \$\begingroup\$ @slepic Using .NET 6, the code as-is can be copied/pasted and run without any changes. I just launched a new project in VS, copied the code in the question, and ran it without issue. Line 1 is the first Console.WriteLine(), and the last line in the file is line 28, the closing brace of the final else if block. If OP is just starting with C#, it's likely that their file only contains what's shown in the question (and perhaps are not aware of namespaces, public static void Main etc.) \$\endgroup\$
    – elmer007
    Commented Apr 24, 2023 at 14:37

1 Answer 1

9
\$\begingroup\$

Your code is good, but there are a few improvements that you could make.

Determining what operation your user wants.

Instead of using if/else, I'd probably use a switch/case statement:

switch userInput
{
    case "+":
        Console.WriteLine(numberOne + numberTwo);
        break;
    case "-":
        Console.WriteLine(numberOne - numberTwo);
        break;
    case "*":
        Console.WriteLine(numberOne * numberTwo);
        break;
    case "/":
        Console.WriteLine(numberOne / numberTwo);
        break;
}

Error handling with user inputs

I'd probably also put a default clause, just in case your user doesn't enter one of + - * /:

switch userInput
{
    ...
    default:
        Console.WriteLine("Sorry, unknown operation.");
        break;
}

I would also use a try/catch block for figuring out what numberOne and numberTwo are:

try
{
    Console.WriteLine("Please select your first number: ");
    double numberOne = Convert.ToDouble(Console.ReadLine());
    
    Console.WriteLine("Please select your second number: ");
    double numberTwo = Convert.ToDouble(Console.ReadLine());
}
catch (FormatException)
{
    Console.WriteLine("Sorry, invalid number."); 
}

You might also want to check whether numberTwo is not zero when dividing:

switch userInput
{
    ...
    case "/":
        if numberTwo == 0
        {
            Console.WriteLine("Sorry, cannot divide by zero.");
        }
        else
        {
            Console.WriteLine(numberOne / numberTwo);
        }

        break;
}
\$\endgroup\$
6
  • 5
    \$\begingroup\$ And while you're at it (it being "error handling") you may want to check that numberTwo is not zero when dividing. \$\endgroup\$
    – CompuChip
    Commented Apr 24, 2023 at 14:03
  • 11
    \$\begingroup\$ As an alternative for the try/catch, you could also use double.TryParse. That would also simplify making the app loop until valid numbers are entered. \$\endgroup\$
    – minnmass
    Commented Apr 24, 2023 at 14:42
  • 1
    \$\begingroup\$ @CompuChip, is this what you mean (I have added a section). \$\endgroup\$ Commented Apr 25, 2023 at 5:15
  • \$\begingroup\$ Don't use try catch for converting numbers. Use double.TryParse instead. Try catch will have an impact on performance in duration each time the users enters an invalid number. \$\endgroup\$ Commented Apr 25, 2023 at 10:13
  • \$\begingroup\$ Also, there's no need to duplicate operations (WriteLine). Logic should only modify the data. So create a result variable. Modify it as many times as you need. Print it once. \$\endgroup\$
    – akinuri
    Commented Apr 25, 2023 at 12:41

Your Answer

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

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