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

Basic C# calculator code

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

Problem

I have not had any experience with coding before, and after beginning to learn I stated working on this calculator which runs different functions on the console. I'd like to hopefully be taught what I could have done differently or easier.

```
{

class MainClass

{

public static void Main (string[] args)

{

double num01, num02, num03, Result,Result2;

Random randy= new Random();

Start:

Console.WriteLine ("How could I help you ?" +

"\n01. Add" +

"\n02. Subract " +

"\n03. Multiply " +

"\n04. Divide " +

"\n05. Square of a number" +

"\n06. Square Root of a number" +

"\n07. Cube of a number" +

"\n08. Cube root" +

"\n09. Cº to Fº " +

"\n10. Fº to Cº " +

"\n11. The Area of a Circle " +

"\n12. Circumference of a Circle" +

"\n13. Area of a Rectangle" +

"\n14. Area of a Square "+

"\n15. Area of a Regular Polygon" +

"\n16. Apothem from one side" +

"\n17. Area of a Parallelogram" +

"\n18. Area of a Trapezoid" +

"\n19. Area of a Triangle" +

"\n20. Area of an Ellipse" +

"\n21. Area of a Rhombus" +

"\n22. Surface Area of a Cube" +

"\n23. Surface Area of a Rectangular Prism" +

"\n24. Surface Area of a Sphere" +

"\n25. Surface Area of a Cylinder" +

"\n26. Surface Area of a Rectangular Pyramid" +

"\n27. Surface Area of a Cone" +

"\n28. Volume of a Cube" +

"\n29. Volume of a Rectangular Prism" +

"\n30. Volume of a Sphere" +

"\n31. Volume of a Cylinder" +

"\n32. Volume of a R

Solution

As mentioned above in comments, you should validate your input... something like:

var input = Console.ReadLine();
int numInput;
if (!int.TryParse(input, out numInput))
{
    Console.WriteLine("'" + input + "' is not a number.");
}
else
{
    // processing numInput
}


All the code is structured in one big method. To improve readability and maintainability, the logic should be splitted to multiple methods (in a first step). For instance, each case should be one method; The generation of the main text should be one method and so on. Normally, if the level ob abstraction changes, a new method should be creaded. That could result in something like:

//... (not fully functional right now... code was only splitted into methods)

WriteAvailableOperations();

int operation = int.Parse (Console.ReadLine());

HandleOperation(operation);

WriteMainMenuOptions();

int mainMenuOption = int.Parse (Console.ReadLine ());

HandleMainMenuOption(mainMenuOption)

// ...


You see that this structure eill not work with goto's (gotos have to be avoided anyway because they result in complicated and hard to read code flows).

So, lets split the code further to make it work again:

enum ContinueBehavior { ShowOperations, ShowMainMenu, Exit }

public static void Main (string[] args)
{
    var state = ContinueBehavior.ShowOperations;

    do
    {
        if (state == ContinueBehavior.ShowOperations)
        {
            ProcessOperations();
        }

        state = ProcessMainMenu();
    } 
    while (state != ContinueBehavior.Exit)
}

private static void ProcessOperations()
{
    WriteAvailableOperations();

    int operation = int.Parse (Console.ReadLine());

    HandleOperation(operation);
}

private static ContinueBehavior ShowMainMenu()
{
    WriteMainMenuOptions();

    var mainMenuOption = int.Parse (Console.ReadLine ());

    return HandleMainMenuOption(mainMenuOption)
}


Probably there is is more elegant way to organize the code, but you get the point... Writing maintainable code means to group code that belongs to the same level of abstraction to single code units like methods or classes.

The next step would be to group code that belongs together. The description for each operation is displayed in the method WriteAvailableOperations but the actual logic is handled in the method HandleOperation. Therefore, if you want to add or remove operations, you have to change 2 different locations. That is a error-prone design because it is possible that you forget one.

To fix that, it make sense to create a class Operation that looks similary to:

public abstract class Operation
{
    public int Number { get; set; }
    public string Description {get; set; }
    public abstract void Run();
}


So you coud create a single operation class for each concrete operation:

public class AddOperation
{
   public AddOperation()
   {
       Description = "Add";
       Number = 1;
   }

   public override void Run()
   {
       Console.WriteLine("You have chosen addition \nPlease insert your first number");
       int num01=double.Parse(Console.ReadLine());
       Console.WriteLine("Alright, now your second number");
       int num02= double.Parse (Console.ReadLine());
       int result= num01 + num02;
       Console.WriteLine(num01+" + " +num02+" equals "+ result );
   }
}


So you have to initialize a list of abstract operations first. The main code can work with that abstractions and must not be modified if an operation changed or if operations were added or removed:

public static void Main (string[] args)
{
    var state = ContinueBehavior.ShowOperations;
    var operations = GetOperations();
    do
    {
        if (state == ContinueBehavior.ShowOperations)
        {
            ProcessOperations(operations);
        }

        state = ProcessMainMenu();
    } 
    while (state != ContinueBehavior.Exit)
}

// ...

public Operation[] GetOperations()
{
    return new Operation[]
    {
    new AddOperation(),
    new SubractOperation(),
    //...
    };
}

public static void WriteAvailableOperations(Operation[] operations)
{
    var operations = String.Join(Environment.NewLine, operations.Select(o => o.Number.ToString("00") + ". " o.Description));
    Console.WriteLine("How could I help you ?");
    Console.WriteLine(operations );
}

public static void HandleOperation(int operationNo, Operation[] operations)
{
    var operation = operations.First(o => o.Number == operationNo);
    operation.Run();
}

Code Snippets

var input = Console.ReadLine();
int numInput;
if (!int.TryParse(input, out numInput))
{
    Console.WriteLine("'" + input + "' is not a number.");
}
else
{
    // processing numInput
}
//... (not fully functional right now... code was only splitted into methods)

WriteAvailableOperations();

int operation = int.Parse (Console.ReadLine());

HandleOperation(operation);

WriteMainMenuOptions();

int mainMenuOption = int.Parse (Console.ReadLine ());

HandleMainMenuOption(mainMenuOption)

// ...
enum ContinueBehavior { ShowOperations, ShowMainMenu, Exit }

public static void Main (string[] args)
{
    var state = ContinueBehavior.ShowOperations;

    do
    {
        if (state == ContinueBehavior.ShowOperations)
        {
            ProcessOperations();
        }

        state = ProcessMainMenu();
    } 
    while (state != ContinueBehavior.Exit)
}

private static void ProcessOperations()
{
    WriteAvailableOperations();

    int operation = int.Parse (Console.ReadLine());

    HandleOperation(operation);
}

private static ContinueBehavior ShowMainMenu()
{
    WriteMainMenuOptions();

    var mainMenuOption = int.Parse (Console.ReadLine ());

    return HandleMainMenuOption(mainMenuOption)
}
public abstract class Operation
{
    public int Number { get; set; }
    public string Description {get; set; }
    public abstract void Run();
}
public class AddOperation
{
   public AddOperation()
   {
       Description = "Add";
       Number = 1;
   }

   public override void Run()
   {
       Console.WriteLine("You have chosen addition \nPlease insert your first number");
       int num01=double.Parse(Console.ReadLine());
       Console.WriteLine("Alright, now your second number");
       int num02= double.Parse (Console.ReadLine());
       int result= num01 + num02;
       Console.WriteLine(num01+" + " +num02+" equals "+ result );
   }
}

Context

StackExchange Code Review Q#128806, answer score: 3

Revisions (0)

No revisions yet.