HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Basic C# calculator (+,-,*,/)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
calculatorbasicstackoverflow

Problem

My 'teacher' requested this problem be solved by parsing the input into a tree based on the order of operations. The class Calc variables are specific to his instruction (except precedence) as well as the getValue() method, everything else is subject to review.

This is my first real program in C# but I'm hoping to receive detailed input on how to make my code meet best practices as closely as possible.

Revised edition: Basic C# calculator (+,-,*,/) - V2

Program.cs

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace calcTree
{
class Program
{
static void Main(string[] args)
{
// Various prompt and response strings
string welcome = "Welcome to the Calculator!";
string instructions = "\nPlease enter a space delimited equation (do not use \"x =\" or parenthesis)" +
"\nor enter \"exit\" to quit:\n";
string exit = "\nGoodbye.";
string error = "\nYour input was invalid.";

int end; // Determines when to exit the program

Console.WriteLine(welcome);
do
{
Console.WriteLine(instructions);

// Read input & allocate parsed_input size
string input = Console.ReadLine();

// Removes spaces from input, turns it into an array, and checks for invalid input (exit value)
string[] parsed_input = removeSpaces(input, out end);

// Exit(1) or restart(2) the loop if input is invalid
if (end != 0)
{
if (end == 1)
Console.WriteLine(exit);
else // 2
Console.WriteLine(error);
}
else
{
// Create logic tree
Calc[] calc = Calc.applyLogic(parsed_input

Solution

Class Program

The removeSpaces() method is violating the single responsibility principle as it

  • remove spaces from the given input



  • checks for the word exit



  • checks if the given expression contains a valid equotation



So these 3 responsibilities should be extracted to 3 methods.

Let us start with removing the spaces.

We just use the Replace() method

private static String RemoveSpaces(String input)
{
    return input.Replace(" ", String.Empty);
}


After we have removed any spaces we check for the word exit

As the instructions are clearly stating, entering the word exit and hitting enter should end the application. What do we need to do ? We only need to check if the last 4 characters of the input are the word exit in all the different writing types.

First we check if the length of the input is at minimum 4, so if the length is smaller than 4 we can't find the word exit.

Then we check if the last 4 characters of the input, converted to lower case is exit. To retrieve the last 4 characters we use the SubString() method. For converting the trimmed input to lower case we use the ToLowerInvariant(). Both of these methods are returning a new string.

private static bool ShouldExit(String input)
{
    if (input.Length < 4) { return false; }
    return input.Substring(input.Length - 4).ToLowerInvariant() == "exit";
}


Update

As mjolka has pointed out in the [chat] , this can be simplified using the EndsWith() method

private static bool ShouldExit(String input)  
{
   return input.EndsWith("exit", StringComparison.OrdinalIgnoreCase);
}


Now let us focus on validating the given input. As we don't have separating spaces anymore the current logic doesn't work anymore.

But the current logic had a flaw/bug in it, as 2 + 5 + 6 + shouldn't be valid.

As the validation of the equotation is related to the calculation, it should be part of the Calc class.

If one of the following condition is true, the equotation is invalid:

  • The given input is null or whithespace



  • The given input contains a char which isn't a number, an operator nor a culture specific decimal point



  • The last char doesn't represent a number



As I don't want to give you the solution, I will give you just hints how to solve

  • String.IsNullOrWhiteSpace() checks if a string is null or contains only whitespace characters.



  • As you already know how to iterate through the chars of a String, you can use the Char.IsNumber() method to check if the char is a number.



You can use the Contains() method with a char array (like myArray.Contains('x');). So providing a operator char array, can be used to check if a non number is an operator.

In the System.Globalization namespace lives the NumberFormatInfo class which has a property NumberDecimalSeparator which provides, depending on the current culture, the decimal separator.

  • Should be solvable by 2.



This method should just return a boolean. So we still need to parse the given input into a string array, to get the same result which the original code did return.

The flow can be like

If the current char is an operator, it can be added to the array (or better use a IList respective List).

If the char is either a number or a decimal separator it can be added to a separate array/List until the current char is an operator. Then it can be added to the former array/List and gets cleared afterwards.

The last step is after the e.g loop to add the remaining number or a decimal separator to the result value.

Please check your other methods also regarding SRP. Like e.g solveAndPrint().

Code Snippets

private static String RemoveSpaces(String input)
{
    return input.Replace(" ", String.Empty);
}
private static bool ShouldExit(String input)
{
    if (input.Length < 4) { return false; }
    return input.Substring(input.Length - 4).ToLowerInvariant() == "exit";
}
private static bool ShouldExit(String input)  
{
   return input.EndsWith("exit", StringComparison.OrdinalIgnoreCase);
}

Context

StackExchange Code Review Q#67054, answer score: 10

Revisions (0)

No revisions yet.