7
\$\begingroup\$

This is a calculator I've made in C#. Is there any way to improve it? Surely there is a way to get rid of all the ifs that are nested inside one another.

namespace CalculatorCA
{
    using System;
    using System.Collections.Generic;
    class Program
    {
        static void Main()
        {
            Dictionary<int, Func<double, double, double>> MyDict =
                new Dictionary<int, Func<double, double, double>>();
            FillDictionary(MyDict);
            bool KeepOn = true;
            while (KeepOn)
            {
                Console.WriteLine("Enter the first number!");
                double first;
                if (double.TryParse(Console.ReadLine(), out first))
                {
                    Console.WriteLine("Please choose one of the following:\n1-Add\n2-Substract\n3-Multiply\n4-Divide");
                    int choice;
                    if (int.TryParse(Console.ReadLine(), out choice))
                    {
                        if (MyDict.ContainsKey(choice))
                        {
                            Console.WriteLine("Enter the second number!");
                            double second;

                            if (double.TryParse(Console.ReadLine(), out second))
                            {
                                Console.WriteLine("The result is : {0}", MyDict[choice](first, second));
                            }
                        }
                        else
                        {
                            Console.WriteLine("Invalid operation!");
                        }
                    }
                }
                Console.WriteLine("Continue?[Y/N]");
                KeepOn = Console.ReadKey().Key == ConsoleKey.Y;
                Console.Clear();
            }
        }
        public static void FillDictionary(Dictionary<int,Func<double,double,double>> myDictionary)
        {
            myDictionary.Add(1, (x, y) => x + y);
            myDictionary.Add(2, (x, y) => x - y);
            myDictionary.Add(3, (x, y) => x * y);
            myDictionary.Add(4, (x, y) => x / y);
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Something is broken with your indentation, feel free to edit the post to fix it. Make sure you replace tabs with spaces before pasting from your IDE! \$\endgroup\$ – Mathieu Guindon Aug 24 '15 at 21:36
7
\$\begingroup\$

That's a pretty concise routine there. If you had to change it, I would add methods to parse the user input so now your program logic is less nested and we get back to the good old principal of each method having a single purpose.

Normally in a simple app that has a menu structure I would say use case logic instead of if statements as this will allow you to easily extend the options later, but your cleaver use of the dictionary of functions does away with this altogether.

namespace CalculatorCA
{
    using System;
    using System.Collections.Generic;
    class Program
    {
        static void Main()
        {
            Dictionary<int, Func<double, double, double>> MyDict =
                new Dictionary<int, Func<double, double, double>>();
            FillDictionary(MyDict);
            bool KeepOn = true;
            while (KeepOn)
            {
                double first = ReadInput("Enter the first number!");
                int choice = ReadInput("Please choose one of the following:\n1-Add\n2-Substract\n3-Multiply\n4-Divide", 1, MyDict.Count);
                double second = ReadInput("Enter the second number!");

                Console.WriteLine("The result is : {0}", MyDict[choice](first, second));


                Console.WriteLine("Continue?[Y/N]");
                KeepOn = Console.ReadKey().Key == ConsoleKey.Y;
                Console.Clear();
            }
        }
        private static double ReadInput(string prompt)
        {
            Console.WriteLine(prompt);
            double userInput;
            if (double.TryParse(Console.ReadLine(), out userInput))
                return userInput;

            Console.WriteLine(" - Sorry, I expected a double, try again");
            return ReadInput(prompt);
        }

        private static int ReadInput(string prompt, int? min = null, int? max = null)
        {
            Console.WriteLine(prompt);
            int userInput;
            if (int.TryParse(Console.ReadLine(), out userInput))
            {
                if (userInput >= min.GetValueOrDefault(int.MinValue) && userInput <= max.GetValueOrDefault(int.MaxValue))
                    return userInput;

                Console.WriteLine(" - Sorry, I expected an integer, between {0} and {1}", min, max);
                return ReadInput(prompt, min, max);

            }

            Console.WriteLine(" - Sorry, I expected an integer, try again");
            return ReadInput(prompt, min, max);
        }

        public static void FillDictionary(Dictionary<int, Func<double, double, double>> myDictionary)
        {
            myDictionary.Add(1, (x, y) => x + y);
            myDictionary.Add(2, (x, y) => x - y);
            myDictionary.Add(3, (x, y) => x * y);
            myDictionary.Add(4, (x, y) => x / y);
        }
    }
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.