C# Intermediate Calculator Program

Question

I recently created an intermediate calculator program, primarily to practice programming in C# but also for use with my college subjects. Any feedback on coding practices I can implement to optimise this program and any other programs I write in the future would be greatly appreciated.

Thanks.

using System;
namespace calculatormessingaroundthingy
{
 class Program
 {
 static void Main()
 {
 Console.WriteLine("Hello World!");
 
 bool loopInt=false; // Sets value of loop variable to false.
 while (loopInt == false) // Causes the program to repeatedly run until the user chooses to stop.
 {
 MessageOptions(); // Calls a procedure which lists the user's options.
 var input = Console.ReadLine();
 int inputInt;
 while ((!int.TryParse(input, out inputInt)) | (!(inputInt>=0 && inputInt<=6))) // Loop repeats while either the user's input can't be passed into an int variable or while the int is not between 0 and 6 inclusive.
 {
 Console.WriteLine("ERROR: Invalid Input");
 MessageOptions();
 input = Console.ReadLine();
 }
 if (inputInt==0) // Input of 0 exits the program
 {
 Console.WriteLine("Goodbye!");
 loopInt = true;
 break;
 }
 FirstInput(inputInt); // Calls a procedure which gets the user's first number, the message depending on the user's previous input.
 var strNum1 = Console.ReadLine();
 double num1;
 while ((!double.TryParse(strNum1, out num1))) // Loop repeats while the user's input can't be passed into a double variable.
 {
 Console.WriteLine("ERROR: Invalid Input");
 FirstInput(inputInt);
 strNum1 = Console.ReadLine();
 }
 SecondInput(inputInt); // Calls a procedure which gets the user's first number, the message depending on the user's previous input
 var strNum2 = Console.ReadLine();
 double num2;
 while ((!double.TryParse(strNum2, out num2))) // Loop repeats while the user's input can't be passed into a double variable.
 {
 Console.WriteLine("ERROR: Invalid Input");
 SecondInput(inputInt);
 strNum2 = Console.ReadLine();
 }
 switch (inputInt) // Passes the user's two numbers into corresponding procedure for a certain mathematical operation.
 {
 // inputInt corresponds to the user's respones to the operation they wish to perform.
 case 1:
 Console.WriteLine(Add(num1, num2));
 break;
 case 2:
 Console.WriteLine(Subtract(num1, num2));
 break;
 case 3:
 Console.WriteLine(Multiply(num1, num2));
 break;
 case 4:
 Console.WriteLine(Divide(num1, num2));
 break;
 case 5:
 Console.WriteLine(Powers(num1, num2));
 break;
 case 6:
 Console.WriteLine(Logarithm(num1, num2));
 break;
 }
 }
 }
 static double Powers(double number, double power) // Raises the first number to the power of the second number and returns the result.
 {
 return Math.Pow(number, power);
 }
 static double Add(double number, double number2) // Adds together both numbers and returns the result.
 {
 return number + number2;
 }
 static double Subtract(double number, double number2) // Subtracts the second number from the first number and returns the result.
 {
 return number - number2;
 }
 static double Multiply(double number, double number2) // Multiplies together both numbers and returns the result.
 {
 return number * number2;
 }
 static double Divide(double number, double number2) // Divides the first number by the second number and returns the result.
 {
 return number / number2;
 }
 static double Logarithm(double number, double number2) // Returns the logarithm of base first number and argument second number.
 {
 return Math.Log(number2, number);
 }
 static public void MessageOptions() // Displays the user's inital options.
 {
 Console.WriteLine();
 Console.WriteLine("-------------------------------------");
 Console.WriteLine("Choose one of the following options: ");
 Console.WriteLine("1. Addition");
 Console.WriteLine("2. Subtraction");
 Console.WriteLine("3. Multiplication");
 Console.WriteLine("4. Division");
 Console.WriteLine("5. Powers");
 Console.WriteLine("6. Logarithms");
 Console.WriteLine("0. Exit");
 Console.WriteLine("-------------------------------------");
 }
 static public void FirstInput(int input) // Displays what number should be entered dependent on the inital input.
 {
 switch (input)
 {
 case 1: case 2: case 3: case 4:
 Console.WriteLine("Enter the first number: ");
 break;
 case 5:
 Console.WriteLine("Enter the base number: ");
 break;
 case 6:
 Console.WriteLine("Enter the logarithm's base: ");
 break;
 }
 }
 static public void SecondInput(int input) // Displays what number should be entered dependenent on the inital input.
 {
 switch (input)
 {
 case 1: case 2: case 3: case 4:
 Console.WriteLine("Enter the second number: ");
 break;
 case 5:
 Console.WriteLine("Enter the exponent: ");
 break;
 case 6:
 Console.WriteLine("Enter the logarithm's argument: ");
 break;
 }
 }
 }
}
```

• Reinderien: calculatormessingaroundthingy - Naming is hard. I don't blame you.

Answer 1

First of all, let's review some semantics that make the program a bit clumsy:

1. while (loopInt == false) > while (!loopInt)
2. Your loopInt variable is useless, in the only occasion you change it to true you also break; so your loop can just be while (true) (which, IMO, really expresses its purpose better - unless you want to stop, it shows you the same interface forever.
3. Spacing. Although that should be enforced by your IDE, it makes reading your code way easier. So, for instance, if (inputInt==0) should really be if (inputInt == 0)
4. Hungarian notation is a variable naming convention that prefixes the variable type before its name. You seem to use something similar (inputInt is of type int). This is not encouraged in C#. Your variable may just as well be called input, and with today's advanced IDEs, you just have to hover over the variable name to see its value. No need to clutter its name with the type suffix. Also, you seem to call your loop variable loopInt where it really should say loopBool.
5. Inconsistent use of var. Either you (1) use it everywhere (2) use it nowhere (3) use it in places where you need to use complex types (e.g. Dictionary<string, List<int>>). You seem to use it sometimes, which really is not critical but a little annoying to look at. I think you should form yourself some guidelines when to use var. If you ask me for the guidelines that I follow, usually if any generics are involved or if the type is a class name WithMoreThanTwoWords, then I use var. Otherwise I stick with the actual type name.
6. Function names should be denoting actions, since functions are entities that are supposed to do stuff. For example, SecondInput, IMO, would be a function that displays a message and returns the input. But it's actually quite unclear what it does. In fact, in your code - it does something different than what I would have thought of. In this particular example, I would call the function ShowSecondInputMessage. Although it's longer, it expresses the purpose of the function better.

Now let's get for things that are more important/about program structure itself:

Since your while ... TryParse logic repeats two times (and might repeat some more times) I would separate it into a function double GetInput(string message) and just call it two times (instead of having that logic twice)

I don't like the FirstInput and SecondInput pattern. I think it limits your functions (for instance, what if you need to add a 10eX function that takes only one parameter, X? If you're accommodated to classes in C#, I think I would use this feature to organize the code (see below).

(Note, the following is only a bunch of ideas. You may take some, all or none of them. It is completely different from your code to enable you (and I) to think more open-mindedly)

Let's create a generic MathOperation class:

abstract class MathOperation
{
 public abstract string Name { get; }
 public virtual string[] InputNames => new[] { "First number", "Second number" };
 protected abstract double Calculate(double[] inputs);
}

That structure will let us accept an arbitrary number of inputs and make custom calculations.

Let's try to start extending it. Write a simple AdditionOperation:

sealed class AdditionOperation : MathOperation
{
 public override string Name => "Addition";
 protected override double Calculate(double[] inputs)
 {
 return inputs[0] + inputs[1];
 }
}

Pay attention to the fact that we can just refer to inputs[0] and inputs[1] inside our Calculate function since we are going to ensure the inputs validity.

Let's write the input function that will retrieve the input from the user. We'll implement it inside the MathOperation class.

 protected double[] GetInputs()
 {
 double[] inputs = new double[InputNames.Length];
 for (int i = 0; i < InputNames.Length; ++i)
 {
 inputs[i] = TakeSingleInput(InputNames[i]);
 }
 return inputs;
 }
 private double TakeSingleInput(string parameterName)
 {
 Console.Write("Please enter value for {0}: ", parameterName);
 string userInput = Console.ReadLine();
 double parsedInput;
 while (!double.TryParse(userInput, out parsedInput))
 {
 Console.Write("Invalid input. Please re-enter number: ");
 userInput = Console.ReadLine();
 }
 return parsedInput;
 }

For the sake of completeness of this class, let's also implement a function that will just "do what the operation does":

 public void Run()
 {
 double[] inputs = GetInputs();
 double result = Calculate(inputs);
 Console.WriteLine("The result: {0}", result);
 }

And now, we still only have that switch (inputInt) that we have to take care of. The If-Else Is a Poor Man’s Polymorphism is a good article I recommend reading.

So, now we'll create a simple Calculator class to manage multiple operations:

class Calculator
{
 private List<MathOperation> Operations = new List<MathOperation>();
 public void AddOperation(MathOperation operation) { Operations.Add(operation); }
 public MathOperation SelectOperation()
 {
 Console.WriteLine("Select an operation:");
 for (int i = 0; i < Operations.Count; ++i)
 {
 Console.WriteLine(Operations[i].Name);
 }
 int i = int.Parse(Console.ReadLine()); // TODO: Error handling (not relevant so I'm not implementing it right now)
 return Operations[i];
 }
}

And then your main loop looks roughly like:

 static void Main(string[] args)
 {
 Calculator c = new Calculator();
 c.AddOperation(new AdditionOperation);
 while (true)
 {
 MathOperation operation = c.SelectOperation();
 operation.Run();
 }
 }

Repeating the disclaimer again, this program is larger and more complex than your simple program. But it contains patterns that are very important for the scalability of your code, which is why I suggest you read through my code examples and try to implement it yourself in order to accommodate yourself to those practices of OOP (which is [currently] ruling paradigm in C#)

• Bahkbar: Hey really appreciate the response. Can definitely see the benefit of a more object-oriented approach and will absolutely try to implement these techniques in my current and future projects.

Answer 2

logically, it's fine. Your coding is better than average beginner. You have used the proper way to validate and parse integers, this is something most of beginners lake, even some advanced programmers still parse without validations, which is a real issue when it comes to coding. Why? simply because it's simple validation that would avoid annoying exceptions.

My notes will just give you more thoughts on how things could be done in different ways according to what you've already learned (I'll try to avoid given advanced techniques, to strengthen your current level and to focus on what you have).

Access Modifiers

You need to use access modifiers more often, and also don't misplace them. For a better code readability purpose.
So this :

static double Powers(double number, double power)

should be :

private static double Powers(double number, double power)

And this :

static public void FirstInput(int input)

Should be :

public static void FirstInput(int input)

Comments

You need to use the proper commenting on your code. use summary comments for methods, classes, properties, and structs. The rest you can use single comment line.

So this :

public static double Powers(double number, double power) // Raises the first number to the power of the second number and returns the result.

Should be :

/// <summary>
/// Raises the first number to the power of the second number and returns the result.
/// </summary>
/// <param name="number"></param>
/// <param name="power"></param>
/// <returns></returns>
public static double Powers(double number, double power)

Also, when you have a long comment, like this :

FirstInput(inputInt); // Calls a procedure which gets the user's first number, the message depending on the user's previous input.

The comment is longer than the action itself. Just do this instead :

// Calls a procedure which gets the user's first number,
// the message depending on the user's previous input.
FirstInput(inputInt); 

Why? you should consider that not every screen is large enough to show all the code at once. So, it would be a good idea to simplify the comments and shorten them to be readable and more helpful.

Conditions and Operators

when dealing with conditions, you must consider readability and simplicity above all things. This would help you to handle even complex conditions with ease, because you'll always try to make it simple and readable. For instance, loopInt is not used probably, because this line :

if (inputInt == 0) // Input of 0 exits the program
{
 Console.WriteLine("Goodbye!");
 loopInt = true;
 break;
}

the issue here is that loopInt = true; is meaningless because of break;. When you break the loop. So, this loopInt == false is not well used, because you can replace it with while (true) and it would work as expected !

Let's check this another condition :

while((!int.TryParse(input, out inputInt)) | (!(inputInt>=0 && inputInt<=6))) {...}

It seems a bit unclear, The issue in this is that whenever you have multiple conditions that you need to invert, either invert the condition itself, or group them in parentheses then invert it. which would be more clear to the naked eye. The best practice is to invert the condition itself if you have control on it, if not, then invert the part you can control to the same result of the other part that you don't have control on (like int.TryParse`). So, to make it more practical, we can apply that in your condition above to be like :

while(!int.TryParse(input, out inputInt) || (inputInt < 0 || inputInt > 6)) {...}

Also, don't use single | operator, as the difference between | and || is that the single | would check each condition even if the first one is true. It's rarely used, because it comes with some performance cost, but it has it has its own cases. However, in most cases along with yours, it's not necessary. So, stick with the usual double operator || for OR and && for AND.

Object-Oriented-Programming (OOP)

C# is an OOP programming language, so you should always try to apply that in your coding. Not only to C# but also to any other OOP programming language.

One way to apply that to your current code is applying Encapsulation and Reusability principles. To do that, you can rethink of your application and divide it into layers based on your code purpose. Currently, your code can be divided into (calculator) and (user interface). The calculator would contain all code that used to calculate the values such as Add, Subtract ..etc.. The user interface is where you handle user interactivity. We can then separate them in separate classes, and then use them. If you see any repetitive code, just move them into a method and reuse them (applying another principle Don't Repeat Yourself AKA DRY principle). Though, you could apply more principles but for simplicity sake I prefer to avoid the rest.

So, what we need to do, is to gather all the needed logic under one roof, then modify them to be easy to expand if needed. For instance, if you need to add a new option, on your current work, you would add a new method then you would do several modifications on your code to include the new method. So, this has to be solved. We need only to add a method and just modify one thing, the rest is auto!. We can can take advantage of enum or Dictionary<int, string> to do that. So, first we need the class as following :

public class Calculator
{
 /// <summary>
 /// Raises the first number to the power of the second number and returns the result.
 /// </summary>
 /// <param name="number"></param>
 /// <param name="power"></param>
 /// <returns></returns>
 public double Powers(double baseNumber, double exponent) 
 {
 return Math.Pow(baseNumber , exponent);
 }
 /// <summary>
 /// Adds together both numbers and returns the result.
 /// </summary>
 /// <param name="number"></param>
 /// <param name="number2"></param>
 /// <returns></returns>
 public double Add(double leftHand , double rightHand)
 {
 return leftHand + rightHand;
 }
 /// <summary>
 /// Subtracts the second number from the first number and returns the result.
 /// </summary>
 /// <param name="number"></param>
 /// <param name="number2"></param>
 /// <returns></returns>
 public double Subtract(double leftHand , double rightHand) 
 {
 return leftHand - rightHand;
 }
 /// <summary>
 /// Multiplies together both numbers and returns the result.
 /// </summary>
 /// <param name="number"></param>
 /// <param name="number2"></param>
 /// <returns></returns>
 public double Multiply(double leftHand , double rightHand) 
 {
 return leftHand * rightHand;
 }
 /// <summary>
 /// Divides the first number by the second number and returns the result.
 /// </summary>
 /// <param name="number"></param>
 /// <param name="number2"></param>
 /// <returns></returns>
 public double Divide(double leftHand , double rightHand) 
 {
 return leftHand / rightHand;
 }
 /// <summary>
 /// Returns the logarithm of base first number and argument second number.
 /// </summary>
 /// <param name="number"></param>
 /// <param name="number2"></param>
 /// <returns></returns>
 public double Logarithm(double number , double nBase)
 {
 return Math.Log(number, nBase);
 }
}

Note the comments and the names of the arguments. All of which, gives you a better view of the code.

Now, we can take advantage of enum. We will use it to describe the functions and have a better readability:

public enum CalculatorOption
{
 Undefined = -1, // in case of invalid inputs
 Exit = 0,
 Addition = 1,
 Subtraction = 2,
 Multiplication = 3,
 Division = 4,
 Power = 5,
 Logarithm = 6
}

Now, all we need is two methods, one to parse string as enum and second one is to get these options as string.

For parsing we can do this :

public bool TryParseOption(string option, out CalculatorOption result)
{
 result = CalculatorOption.Undefined;
 if(int.TryParse(option, out int resultInt))
 {
 if(Enum.IsDefined(typeof(CalculatorOption) , resultInt))
 {
 result = (CalculatorOption) resultInt;
 return true;
 }
 }
 else
 {
 return Enum.TryParse<CalculatorOption>(option, true, out result);
 }
 return false;
}

Here I gave the option to parse either by an int or string which means, you can either pass the value or the name of the enum. Example,

// Let's say we need subtraction
CalculatorOption result1; 
CalculatorOption result2; 
var isValidByValue = TryParseOption("2", out CalculatorOption result1);
var isValidByName = TryParseOption("Subtraction", out CalculatorOption result2);
Console.WriteLine(result1 == result2); // True

Now, we need to list the CalculatorOption values, we will use Linq to do that (some Linq won't hurt though, it's a good way of learning).

public string GetOptionsAsString()
{
 var options = Enum.GetValues(typeof(CalculatorOption))
 .Cast<CalculatorOption>()
 .Where(x=> x != CalculatorOption.Undefined)
 .Select(x=> $"{(int)x}. {x}");
 return string.Join(Environment.NewLine , options);
}

What is happening above is that we've accessed the enum and get all members under that enum, excluded Undefined from the list, because it's going to be used for the application not the user. Then, we iterate over each element in the enumeration using Select to convert it to string. Finally, we join these elements using string.Join into one string to be presented to the user.

Finally, we need a method to calculate based on the option, So we can add the following method into the Calculator:

public double Calculate(CalculatorOption option, double firstNumber , double secondNumber)
{
 switch(option)
 {
 case CalculatorOption.Addition:
 return Add(firstNumber , secondNumber);
 case CalculatorOption.Subtraction:
 return Subtract(firstNumber , secondNumber);
 case CalculatorOption.Multiplication:
 return Multiply(firstNumber , secondNumber);
 case CalculatorOption.Division:
 return Divide(firstNumber , secondNumber);
 case CalculatorOption.Power:
 return Powers(firstNumber , secondNumber);
 case CalculatorOption.Logarithm:
 return Logarithm(firstNumber , secondNumber);
 default:
 return 0;
 }
}

Now, you only need to change your Program class to include the changes as following:

public class Program
{
 private static readonly Calculator _calculator = new Calculator();
 static void Main(string[] args)
 {
 Console.WriteLine("Hello World!");
 while(true)
 {
 PrintOptions();
 var inputOption = GetSelectedOption(Console.ReadLine());
 if(inputOption == CalculatorOption.Exit)
 {
 Console.WriteLine("Goodbye!");
 break;
 }
 Console.WriteLine("Enter the first number: ");
 var firstInput = TryParseInput(Console.ReadLine());
 Console.WriteLine("Enter the second number: ");
 var secondInput = TryParseInput(Console.ReadLine());
 var result = _calculator.Calculate(inputOption , firstInput , secondInput);
 Console.WriteLine();
 Console.WriteLine($"Result = {result}");
 }
 Console.ReadLine();
 }
 private static void PrintOptions()
 {
 Console.WriteLine();
 Console.WriteLine("-------------------------------------");
 Console.WriteLine("Choose one of the following options: ");
 Console.WriteLine(_calculator.GetOptionsAsString());
 Console.WriteLine("-------------------------------------");
 }
 private static double TryParseInput(string input)
 {
 double result;
 while(!double.TryParse(input , out result))
 {
 Console.WriteLine("ERROR: Invalid Input");
 Console.WriteLine("Please enter a valid integer");
 input = Console.ReadLine();
 }
 return result;
 }
 private static CalculatorOption GetSelectedOption(string input)
 {
 CalculatorOption result;
 while(!_calculator.TryParseOption(input , out result) || result == CalculatorOption.Undefined)
 {
 Console.WriteLine("ERROR: Invalid Input");
 PrintOptions();
 input = Console.ReadLine();
 }
 return result;
 }
}

Now, let's say you want to add Max function to the list, all you need to do is to add Max = 7 to the enum and add the method, then adjust Calculate method to include the new method. This would be it.

As I mentioned, I've tried to avoid advanced techniques, which you'll learn in the future, however, for better expandability you'll need to learn about inheritance and design patterns along with what you've learned. To overcome the design issues.

• Bahkbar: Hey thanks a lot for the response. Yeah sorry about the commenting mess, my college wants us to comment every piece of code we write but never really said how to so thanks. I think this might be slightly too advanced for me at the moment, but I'll definitely bear what you've said in mind and try to implement said techniques where possible.