Simple console calculator app. Looking for structure/best practices tips

Question

I'm very much new to coding. The template for a .NET Console app opens up blank. I added lines like

using System.ComponentModel.Design;

internal class Program

private static void Main(string[] args)

despite not knowing the purpose or necessity. I'd really just like to know what I can change to make this a bit better. As well as learning best practices for future reference.

using System.ComponentModel.Design;
internal class Program
{
 private static void Main(string[] args)
 {
 Console.WriteLine("Hello! Welcome to the calculator!");
 Console.WriteLine(" ");//Creates blank line
 Console.WriteLine("Please enter the first number.");
 Console.WriteLine(" ");//Creates blank line
 string firstRes = 
 Console.ReadLine();
 int firstNum = Int32.Parse(firstRes);
 Console.WriteLine(" ");//Creates blank line
 Console.WriteLine("Please enter the second number.");
 Console.WriteLine(" ");//Creates blank line
 string secRes = 
 Console.ReadLine();
 int secNum = Int32.Parse(secRes);
 Console.WriteLine(" ");//Creates blank line
 Console.WriteLine("What operator would you like to use?");
 Console.WriteLine(" ");//Creates blank line
 string opRes = 
 Console.ReadLine();
 Console.WriteLine(" ");//Creates blank line
 int sysResp;
 if (opRes == "+")
 {
 sysResp = firstNum + secNum;
 Console.WriteLine("The sum of these numbers is : " + sysResp);
 }
 
 
 else if (opRes == "-")
 {
 sysResp = firstNum - secNum;
 Console.WriteLine("The difference of these numbers is : " + sysResp);
 }
 else if (opRes == "*")
 {
 sysResp = firstNum * secNum;
 Console.WriteLine("The product of these numbers is : " + sysResp);
 }
 else if (opRes == "/")
 {
 sysResp = firstNum / secNum;
 Console.WriteLine("The quotient of these numbers is : " + sysResp);
 }
 else if (opRes == "%") 
 {
 sysResp = firstNum % secNum;
 Console.WriteLine(secNum + " is " + sysResp + " of " + firstNum);
 }
 
 else
 {
 Console.WriteLine("That is not a valid operator. Please try again. ");
 Console.WriteLine(" ");
 }
 }
}

• Chris: There's a lot of extra empty lines, and the indentation needs work. You spend more time reading code than writing it.
• Chris: You never check that your input actually are integers. What happens if Int32.Parse fails?
• Alexander Petrov: Try writing unit tests. Try not to cry.
• Jesse C. Slicer: Two quickie items: Console.WriteLine(" ");//Creates blank line 1) the comment is wrong as it creates a line with a single space; what you want is Console.WriteLine() if you want just a blank line. 2) The comment is pretty useless. Comments should tell "why" or "how", not "what". The code tells the "what".
• radarbob: That is not a valid operator -> What is not a valid operator? Show this errant op in the message. "Well, Bob, just look at what you entered." Bob: "What do you think I entered?"

Answer 1

There are many ways how it could be improved but let me focus on a single aspect in this post: reduce repetitive code for better reusability and maintainability. I suggest to try to visualize the logical steps of your application to better understand what's going on:

application steps

I've just put together a simple state diagram with mermaid. From this we can see that the get x operand and the calculate the result steps are good candidates for further investigation.

Get x operand

Console.WriteLine(" ");//Creates blank line
Console.WriteLine("Please enter the first number.");
Console.WriteLine(" ");//Creates blank line
string firstRes = 
 Console.ReadLine();
 int firstNum = Int32.Parse(firstRes);

Console.WriteLine(" ");//Creates blank line
Console.WriteLine("Please enter the second number.");
Console.WriteLine(" ");//Creates blank line
string secRes = 
 Console.ReadLine();
int secNum = Int32.Parse(secRes);

As you can see we are doing the very same thing twice:

• Print some instruction
• Ask for user input
• Parse the input as number

We can introduce a function to avoid code duplication.
We can use parameters to customize each invocation:

static int GetNthOperand(string nth)
{
 string newLine = Environment.NewLine;
 Console.WriteLine($"{newLine}Please enter the {nth} number{newLine}");
 string input = Console.ReadLine();
 return Int32.Parse(input);
} 

Then the usage will be this simple:

int firstOperand = GetNthOperand("first"); //or left-hand-side operand
int secondOperand = GetNthOperand("second"); //or right-hand-side operand

Calculate the result

if (opRes == "+")
{
 sysResp = firstNum + secNum;
 Console.WriteLine("The sum of these numbers is : " + sysResp);
}
else if (opRes == "-")
{
 sysResp = firstNum - secNum;
 Console.WriteLine("The difference of these numbers is : " + sysResp);
}
else if (opRes == "*")
{
 sysResp = firstNum * secNum;
 Console.WriteLine("The product of these numbers is : " + sysResp);
}
else if (opRes == "/")
{
 sysResp = firstNum / secNum;
 Console.WriteLine("The quotient of these numbers is : " + sysResp);
}
else if (opRes == "%") 
{
 sysResp = firstNum % secNum;
 Console.WriteLine(secNum + " is " + sysResp + " of " + firstNum);
}

Here we preform the calculation and then we print the result. The result and the message are both unique for each operator. So, we can do something like this to make the code more concise:

var (result, message) = arithmeticOperator switch
{
 "+" => (firstOperand + secondOperand, "The sum of these numbers is : {0}"),
 "-" => (firstOperand - secondOperand, "The difference of these numbers is : {0}"),
 "*" => (firstOperand * secondOperand, "The product of these numbers is : {0}"),
 "/" => (firstOperand / secondOperand, "The quotient of these numbers is : {0}"),
 "%" => (firstOperand % secondOperand, secondOperand + " is {0} of " + firstOperand)
};
Console.WriteLine(message, result);

The switch expression calculates the result and a message template. The {0} represents a placeholder which will be replaced with the calculated result when we print the message.

---

I think it is worth mentioning that you should pay more attention to error handling as well. Input data can be anything not just numbers so Int32.Parse could potentially fail; dividing by zero could also cause runtime issues; etc..

Answer 2

I'm very much new to coding ... snipped for brevity ... I'd really just like to know what I can change to make this a bit better. As well as learning best practices for future reference.

Since you have in all humility asked for help to improve yourself, I think you are quite deserving of patient explanations.

The answer from Peter Csala mainly focuses on one major aspect of coding. Peter is teaching about the Don't Repeat Yourself or DRY Principle for the benefit of reduce repetitive code for better reusability and maintainability (using Peter's own words).

Peter and Chris in the comments also ask you what happens if someone enters letters instead of an integer number. Keyboard mistakes do happen. I would refer you to the int.TryParse(string, out int number) method. Note the method returns a Boolean denoting whether the parsing to integer was successful.

Jessie C Slicer had 2 excellent points in the comments that should have been their own answer. Do not dismiss them because of the comment. Heed them well.

Others have remarked about you have way too many blank lines, or that your variable naming could be better.

So far I have just repeated things said by others and offered nothing original. Let me change that.

For the small bit of code you posted, I see no reason for:

using System.ComponentModel.Design;

You also said you needed to add:

internal class Program
private static void Main(string[] args)

I am guessing you are using .NET 6 or later. I am old school back from .NET Framework days, and my eyes just want to see those lines too. However, with .NET 6+, they are not needed. Under the covers, the dotnet compiler will add them for you in your Program.cs file.

For beginners, there seems to be a tendency to clump all their code into Program.cs. As your code base gets larger, you want to avoid this. A general rule of thumb is that your Program class should be as minimal as possible. What you have works for now, but do not mistake it as being acceptable always.

Malachi's answer mentions using Environment.NewLine instead of many Console.WriteLine() calls. This is advice in the right direction but I would skip Environment.NewLine and instead use \n:

Console.WriteLine($"\nPlease enter the {nth} number\n");

I prefer this for console writing. However, if you are writing to a text file, then you should prefer Environment.NewLine. Why? Depends on Environment, that's why! You see for Windows, the new line is CarriageReturn + LineFeed characters, ASCII 13 & 10 respectively, also commonly called CRLF. But for Linux environments, Environment.NewLine returns "\n", or ASCII 10, which is simply LineFeed.

Peter used the DRY Principle. There are lots of other good principles of which to be aware. Some of them:

SRP - Single Responsibility Principle. To keep code maintainable, methods should do one thing. This makes sense on smaller levels, such as prompting for integer input. Eventually you will need a method that ties a lot of the smaller pieces together, in which case one would argue the SRP for that method is to server as an orchestrator relating many methods together.

Principle of Least Astonishment - this could be related to good naming or back to SRP. For example, you do not want a method called Subtract when it would actually multiply the numbers together. A common violation I see is a method named GetFoo and before returning the foo, it also sets bar.

It all touches upon what Peter said: you want small, maintainable, reusable methods that act like building blocks.

• radarbob: ... serves as an orchestrator relating many methods together -> I see 3 distinct SRP methods in @petercsala flow chart: UI interaction for input; calculator-ing; UI interaction for output.

Answer 3

I would like to expand on what Peter wrote for GetNthOperand

string input = Console.ReadLine();
return Int32.Parse(input);

This part, I would change this so that it is a single line,

return Int32.Parse(Console.ReadLine());

I haven't tested this, you may have to do something different with the Int32 part, I don't write code on a regular basis much anymore.

so this,

static int GetNthOperand(string nth)
{
 string newLine = Environment.NewLine;
 Console.WriteLine($"{newLine}Please enter the {nth} number{newLine}");
 string input = Console.ReadLine();
 return Int32.Parse(input);
} 

would turn into this:

static int GetNthOperand(string nth)
{
 string newLine = Environment.NewLine;
 Console.WriteLine($"{newLine}Please enter the {nth} number{newLine}");
 return Int32.Parse(Console.ReadLine());
}