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;
}
}
}
-
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\$Sᴀᴍ Onᴇᴌᴀ– Sᴀᴍ Onᴇᴌᴀ ♦2020年12月30日 17:57:29 +00:00Commented Dec 30, 2020 at 17:57
2 Answers 2
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
.
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.