5
\$\begingroup\$

The program takes 2 decimals as input and an operator (+, -, *, /, ^) so far. Any suggestions to make this code cleaner or shorter?

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

namespace Calculator
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.Write("Enter a number: ");
            Decimal n1 = Convert.ToDecimal(Console.ReadLine());

            Console.Write("Enter an operator: ");
            string op = Console.ReadLine();

            Console.Write("Enter a number: ");
            Decimal n2 = Convert.ToDecimal(Console.ReadLine());

            if (op == "+")
            {
                Console.WriteLine(Sum(n1, n2));
                Console.ReadLine();
            }

            else if (op == "-")
            {
                Console.WriteLine(Dif(n1, n2));
                Console.ReadLine();
            }

            else if (op == "*")
            {
                Console.WriteLine(Prod(n1, n2));
                Console.ReadLine();
            }

            else if (op == "/")
            {
                Console.WriteLine(Div(n1, n2));
                Console.ReadLine();
            }

            else if (op == "^")
            {
                Console.WriteLine(Pow(n1, n2));
                Console.ReadLine();
            }

            else
            {
                Console.WriteLine("Invalid Operator! Only '+', '-', '*', '/', or '^' allowed.");
                Console.ReadLine();
            }
        }

        static Decimal Sum(Decimal n1, Decimal n2)
        {
            Decimal sum;
            sum = n1 + n2;
            return sum;
        }

        static Decimal Dif(Decimal n1, Decimal n2)
        {
            Decimal dif;
            dif = n1 - n2;
            return dif;
        }

        static Decimal Prod(Decimal n1, Decimal n2)
        {
            Decimal prod;
            prod = n1 * n2;
            return prod;
        }

        static Decimal Div(Decimal n1, Decimal n2)
        {
            Decimal div;
            div = n1 / n2;
            return div;
        }

        static Decimal Pow(Decimal n1, Decimal n2)
        {
            Decimal pow;
            pow = (decimal)Math.Pow((double)n1, (double)n2);
            return pow;
        }
    }
}
\$\endgroup\$
1
  • 4
    \$\begingroup\$ Welcome to Code Review! I rolled back your last edit. After getting an answer you are not allowed to change your code anymore. This is to ensure that answers do not get invalidated and have to hit a moving target. If you have changed your code you can either post it as an answer (if it would constitute a code review) or ask a new question with your changed code (linking back to this one as reference). See the section What should I not do? on What should I do when someone answers my question? for more information \$\endgroup\$ Dec 30 '20 at 17:57
3
\$\begingroup\$

Remove the useless functions

There's already a sum function. It's called +

Instead of

Console.WriteLine(Sum(n1, n2));

and

static Decimal Sum(Decimal n1, Decimal n2)
{
    Decimal sum;
    sum = n1 + n2;
    return sum;
}

just write

Console.WriteLine(n1 + n2);

... and the exact same thing applies to all the other calculation functions you wrote.

The same thing in every branch of an if

No matter what op is, you do Console.ReadLine();. So delete it from each if/else if/else and just add it after the end of the if.

\$\endgroup\$
0
\$\begingroup\$

In addition to what the accepted answer states:

You can convert all if statements to a single switch statement. I would also parse the input more defensively, i.e. use decimal.TryParse() and break execution early if the input is invalid.

\$\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.