Code Review or Best Practices thread

Secure Da Bag

Veteran
Joined
Dec 20, 2017
Messages
41,304
Reputation
21,358
Daps
129,492
I don't think my code is SOLID enough. The if statement in the run( ) function really bothers me.

Code:
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace CalculatorConsoleApp
{
    public class Calculator
    {
        public Calculator()
        {

        }

        public void run()
        {
            Expression equation = new Expression();
            string symbol;

            Console.WriteLine("Enter one digit or arithmetic operator: ");
            ConsoleKeyInfo readKey = Console.ReadKey();
            symbol = readKey.KeyChar.ToString();
            while (symbol != "x")
            {

                // check symbol type
                // put in proper object
                // add to expression
                // check if symbol is "=", if so evaluate
                
                InputSymbol input = new InputSymbol(symbol);
                ISymbol expressionSymbol;

                if (input.isCommand())
                {
                    expressionSymbol = new Command(input.Value);
                }
                else if (input.isNumber())
                {
                    expressionSymbol = new Number(input.Value);
                }
                else if (input.isOperation())
                {
                    expressionSymbol = new Operation(input.Value);
                }
                else
                {
                    expressionSymbol = new EmptySymbol();
                }

                if (input.Value == "=")
                {
                    Console.WriteLine();
                    Console.WriteLine(equation.evaluate());
                }
                else
                {
                    equation.buildExpression(expressionSymbol);
                    Console.WriteLine();
                    Console.WriteLine(equation.createStringExpression());
                }

                // get new input
                Console.WriteLine("Enter one digit or arithmetic operator: ");
                readKey = Console.ReadKey();
                symbol = readKey.KeyChar.ToString();
                Console.WriteLine();
            }
        }

        
    }

    public interface ISymbol
    {
        string Value { get;  }
    }

    public class Expression
    {
        StringBuilder stringExpression;
        List<ISymbol> symbolExpression;

        public Expression()
        {
            stringExpression = new StringBuilder();
            symbolExpression = new List<ISymbol>();
        }

        public float evaluate()
        {
            float? result = null;

            if (symbolExpression.Last() is Operation)
            {
                // do nothing
            }
            else
            {
                Operation temp = null;
                foreach(var se in symbolExpression)
                {
                    if (result is null)
                    {
                        result = float.Parse(se.Value);
                    }
                    else if (se is Operation)
                    {
                        temp = se as Operation;
                    }
                    else if (se is Number)
                    {
                        if (temp.Value == "+")
                        {
                            result += float.Parse(se.Value);
                        }
                        else if (temp.Value == "-")
                        {
                            result -= float.Parse(se.Value);
                        }
                        else if (temp.Value == "*")
                        {
                            result *= float.Parse(se.Value);
                        }
                        else if (temp.Value == "/")
                        {
                            result /= float.Parse(se.Value);
                        }
                    }
                }

            }

            symbolExpression.Clear();
            stringExpression.Clear();

            return result.Value;

        }

        public void buildExpression(ISymbol expressionSymbol)
        {
            if (expressionSymbol is EmptySymbol)
            {
                // do nothing
            }
            else if (!symbolExpression.Any() && expressionSymbol is Number)
            {
                symbolExpression.Add(expressionSymbol);
            }
            else if (symbolExpression.Last() is Number && expressionSymbol is Operation)
            {
                symbolExpression.Add(expressionSymbol);
            }
            else if (symbolExpression.Last() is Operation && expressionSymbol is Number)
            {
                symbolExpression.Add(expressionSymbol);
            }
            else
            {
                if (expressionSymbol is Command)
                {
                    var operationSymbol = expressionSymbol as Command;
                    if (operationSymbol.Value == "b")
                    {
                        symbolExpression.RemoveAt(symbolExpression.Count - 1);
                    }
                    else if (operationSymbol.Value == "c")
                    {
                        symbolExpression.Clear();
                    }
                }

                if (expressionSymbol is Operation)
                {
                    var operationSymbol = expressionSymbol as Operation;
                    if (operationSymbol.Value == "=")
                    {

                    }
                }
            }
        }

        public string createStringExpression()
        {
            stringExpression.Clear();
            foreach (var se in symbolExpression)
            {
                stringExpression.Append(se.Value);
            }

            return stringExpression.ToString();
        }
    }

    public class EmptySymbol : ISymbol
    {
        public string Value { get; private set; }

        public EmptySymbol()
        {
            Value = string.Empty;
        }
    }

    public class Command : ISymbol
    {

        public string Value { get; private set; }
        public Command(string symbol)
        {
            Value = symbol;
        }

        
    }

    public class Operation : ISymbol
    {
        public string Value { get; private set; }
        
        public Operation(string symbol)
        {
            Value = symbol;
        }
    }

    public class Number : ISymbol
    {
        public string Value { get; private set; }
        
        public Number(string symbol)
        {
            Value = symbol;
        }
    }

    public class InputSymbol : ISymbol
    {
        public string Value { get; private set; }

        public InputSymbol(string symbol)
        {
            Value = symbol;
        }

        public bool isNumber()
        {
            int output = 0;
            bool result = int.TryParse(Value, out output);
            return result;
        }

        public bool isOperation()
        {
            bool result;
            switch (Value)
            {
                case "/":
                case "*":
                case "+":
                case "-":
                case "%":
                case "=":
                    result = true; break;
                default:
                    result = false; break;
            }

            return result;
        }

        public bool isCommand()
        {
            bool result;
            switch (Value)
            {
                case "b":
                case "c":
                case "x":
                    result = true; break;
                default:
                    result = false; break;
            }

            return result;
        }
    }
}

It works though.
 
Last edited:
Top