14
\$\begingroup\$

I just started learning C#, and I don't want to use any Youtube tutorials if not necessary. I made a basic calculator, but I feel like I could've used more of the advantages of C#. And what I did that is inefficient?

Console.WriteLine("Enter the first number: ");
int a = Convert.ToInt32(Console.ReadLine());

Console.WriteLine("Enter the second number: ");
int b = Convert.ToInt32(Console.ReadLine());


static void  Plus(int a, int b)
{
    int c = a + b;
    Console.WriteLine(a + "+" + b + " = " + c);
}

static void Minus(int a, int b)
{
    int c = a - b;
    Console.WriteLine(a + "-" + b + " = " + c);
}

static void  Times(int a, int b)
{
    int c = a * b;
    Console.WriteLine(a + "+" + b + " = " + c);
}

static void  Divide(int a, int b)
{
    int c = a / b;
    Console.WriteLine(a + "/" + b + " = " + c);
}

Console.WriteLine("Which operation? (+, -, *, /)");
string op = Console.ReadLine() ??"";

if (op == "+")
{
    Plus(a, b);
}
else if (op == "-")
{
    Minus(a, b);
}
else if (op == "*")
{
    Times(a, b);
}
else if (op == "/")
{
    Divide(a, b);
}
else
{
    Console.WriteLine("Incorrect operation entry!");
}

I used things like "static" and "void" but didn't use a "class"; does adding class make it better and how can it make it better?

Or I used "WriteLine" and "ReadLine" separately; is there a better way to output a text and getting an input?

I heard it's an object-oriented programming language; is the class and object just a function like "def" in python?

\$\endgroup\$
0

3 Answers 3

6
\$\begingroup\$

The other answers already give very good advice, but let's dig a bit deeper.

I used things like "static" and "void" but didn't use a "class"; does adding class make it better and how can it make it better?

Classes and other features like namespaces help to organize and reuse code. Every language has some features that help in this regard. For such a simple example you can get away without them, but once a program has a lot more functionality, uses external libraries - or is used itself by other programs - these become important. C# used to require that everything is placed in namespaces and classes, over the last years some of it became optional to facilitate simple programs and scripts. Under the hood, all these are still there and just get automatically generated for you.

Or I used "WriteLine" and "ReadLine" separately; is there a better way to output a text and getting an input?

For such a simple questions-answer "game" this is totally fine. Command line pograms usually are made so that they can perform their job without requiring input during execution by reading everything they need from arguments given to them at the start. A calculator could for example be called like Calculate.exe 42+23 and then interprete what it is asked to do.

Another path forward is to build a graphical user interface (GUI) for the calculator. This is why Jesse pointed out that your calculation methods should not handle output as well. It would just not work when used in another context.

Let's look at a few other things we can do with the code. None of these are required, but they might point you toward possible paths forward. I'll just explain it in the code via comments.

// First: Well, comments.
// It's hard to find the sweet spot. You'll want to help others (or yourself)
// explain why you did something, but not what you do, that's what the code is for.

// As explained, these computation methods should not handle output to the console.
// We'll do that further down.
//
// Your original version used int as type for numbers.
// That works well with most computations, but gives strange results for division.
// Let's use double instead. It has other drawbacks, but is much closer to what
// you would expect a calculator to do.
//
// And such small methods that just return a value can be written in a
// very short way in C#:
static double Plus(double a, double b) => a + b;
static double Minus(double a, double b) => a - b;
static double Times(double a, double b) => a * b;
static double Divide(double a, double b) => a / b;

// You ask for a number, but what happens when the user enters "two"?
// Your program just crashes. That's not nice. So we do some basic error handling.
// We'll come back to it later.
try
{
    // As input we still only allow int(eger) numbers, also known as Int32.
    // But C# let's us convert these to doubles just by declaring the variable it
    // gets assigned to as such. There are many other methods to do that,
    // but for now this is fine.
    Console.WriteLine("Enter the first number: ");
    double a = Convert.ToInt32(Console.ReadLine());

    Console.WriteLine("Enter the second number: ");
    double b = Convert.ToInt32(Console.ReadLine());

    Console.WriteLine("Which operation? (+, -, *, /)");
    string op = Console.ReadLine() ?? "";

    // Using "switch" to simplify a lot of "if/else" checks was already mentioned.
    // This here is a shorter version that can be used in simple cases.
    // And in case we don't recognize the operator, we throw an exception to
    // indicate something bad happened. This is part of the error handling
    // and will be dealt with further down.
    var c = op switch
    {
        "+" => Plus(a, b),
        "-" => Minus(a, b),
        "*" => Times(a, b),
        "/" => Divide(a, b),
        _ => throw new NotSupportedException("Incorrect operation entry!"),
    };

    // The output is handled the same way for all computations.
    // There are a lot of ways to create strings from data, here we do
    // string interpolation with formatting. See the ":F" after the "c"?
    // This indicates that we want to format the number with a maximum of
    // 2 digits after the decimal separator. I always have to look these up ...
    //
    // By the way: when an exception is thrown above, we won't even get here.
    // Instead we jump directly to the catch statement below.
    Console.WriteLine($"{a}{op}{b} = {c:F}");
}
catch (Exception ex)
{
    // We only enter this catch block when an exception was thrown inside the 
    // try block above. Could be that someone entered text instead of a number
    // (Convert.ToInt32 would then throw a "FormatException"),
    // could be that they typed in an operator we don't support.
    // This way the user knows what happens and can do better the next time.
    Console.WriteLine($"Error: {ex.Message}");
}

Without the comments this is much shorter than your version and has basic error handling. But it is still easy to read ... once you get used to C#.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ As for using objects: that can be refactored later, when it becomes necessary. I've seen a great talk where somebody did all kinds of interesting refactoring in the first half of a talk, spending the final half of the talk explaining why all the additional patterns only complicated the program unnecessarily. As long as the application is well thought out, readable and concise (in that order) the code should be fine. \$\endgroup\$ Commented Nov 26 at 14:33
21
\$\begingroup\$

There are a bunch of things that could be touched upon, but given you're a beginner, I would like to show a couple things. One is a language-technical thing, while the other is an overall design thing. Let's start with the second, which is called in programming circles as separation of concerns. What is a concern? File IO, calculations, interacting with the user, accessing a web service, etc. In your code's case, the calculations and interacting with the user are combined together in the local methods, so let's use a quick little language feature known as return types to let the calculations do ONLY calculations:

static int  Plus(int a, int b)
{
    int c = a + b;
    return c;
}

static int Minus(int a, int b)
{
    int c = a - b;
    return c;
}

static int  Times(int a, int b)
{
    int c = a * b;
    return c;
}

static int  Divide(int a, int b)
{
    int c = a / b;
    return c;
}

now this code is reusable in other domains in which you are not interacting with the user via Console.WriteLine. Now because of that, calling code needs to be adjusted somewhat:

Console.WriteLine("Which operation? (+, -, *, /)");
string op = Console.ReadLine() ??"";
int c;

if (op == "+")
{
    c = Plus(a, b);
    Console.WriteLine(a + "+" + b + " = " + c);
}
else if (op == "-")
{
    c = Minus(a, b);
    Console.WriteLine(a + "-" + b + " = " + c);
}
else if (op == "*")
{
    c = Times(a, b);
    Console.WriteLine(a + "+" + b + " = " + c);
}
else if (op == "/")
{
    c = Divide(a, b);
    Console.WriteLine(a + "/" + b + " = " + c);
}
else
{
    Console.WriteLine("Incorrect operation entry!");
}

This is keeping your user interaction in one place. However, there seems to be quite a bit of repeated code here, and there's a principle called DRY (Don't Repeat Yourself) we can implement using a C# feature known as a switch statement. Here's the adjustment now:

Console.WriteLine("Which operation? (+, -, *, /)");
string op = Console.ReadLine() ??"";
int c;

switch (op)
{
    case "+":
        c = Plus(a, b);
        break;
    case "-":
        c = Minus(a, b);
        break;
    case "*":
        c = Times(a, b);
        break;
    case "/":
        c = Divide(a, b);
        break;
    default:
        Console.WriteLine("Incorrect operation entry!");
        return;
}

Console.WriteLine(a + op + b + " = " + c);

This took the four ever so slightly different Console.WriteLines and combined them into one (since we had op in hand).

Now there are many other ways to refine the code, taking advantage of more object-oriented principles, but I think these two items make your entry-level application tight.

BONUS (ETA 2024-Dec-02) - here's a more object-oriented solution using a base class and subclasses if you feel you want to further separate your logic into classes and subclasses (C#12 syntax, I believe):

Console.WriteLine("Enter the first number: ");
int a = Convert.ToInt32(Console.ReadLine());

Console.WriteLine("Enter the second number: ");
int b = Convert.ToInt32(Console.ReadLine());

Console.WriteLine("Which operation? (+, -, *, /)");
string op = Console.ReadLine() ?? string.Empty;
object? operation = op switch
{
    "+" => new Plus(a, b),
    "-" => new Minus(a, b),
    "*" => new Times(a, b),
    "/" => new Divide(a, b),
    _ => null
};

Console.WriteLine(operation ?? "Incorrect operation entry!");

public abstract class Operation(int a, int b, string op, Func<int, int, int> operation)
{
    private readonly string _op = !string.IsNullOrWhiteSpace(op) ? op : throw new ArgumentNullException(nameof(op));

    private readonly Func<int, int, int> _operation = operation ?? throw new ArgumentNullException(nameof(operation));

    public override string ToString() => $"{a}{_op}{b} = {_operation(a, b)}";
}

public sealed class Plus(int a, int b) : Operation(a, b, "+", (ay, be) => ay + be);

public sealed class Minus(int a, int b) : Operation(a, b, "-", (ay, be) => ay - be);

public sealed class Times(int a, int b) : Operation(a, b, "*", (ay, be) => ay * be);

public sealed class Divide(int a, int b) : Operation(a, b, "/", (ay, be) => ay / be);
\$\endgroup\$
2
  • \$\begingroup\$ If your going to simplify everything but the basic arithmetic operations out of the function, wouldn't it be better to just use the operators + - * / without those functions at all? Unless you intend to do something with delegates, which you don't, keeping those redundant functions is only adding complexity and reducing syntactic sugar. \$\endgroup\$ Commented Nov 24 at 20:58
  • 7
    \$\begingroup\$ @BrendanLynn until you want to specify a ¡ b as a!^b in your calculator and get long implementations, want cashing, are using interfaces etc etc. It's indeed useless in this specific case, but as it's about learning, who knows where the ideas will be applied 🤷‍♂️ \$\endgroup\$ Commented Nov 24 at 21:26
9
\$\begingroup\$

Besides Jesse's great answer, you could use string interpolation. Instead of writing:

Console.WriteLine(a + "+" + b + " = " + c);

You can write:

Console.WriteLine($"{a}+{b} = {c}");

This makes it easier to parse and understand what the final result will be, especially if you get longer and/or more complicated strings. For example, you can now clearly see that you have spaces around your equals sign, but not your operators.

(Also, you have a very minor mistake. In your Times() method, you use a '+' in your Console.Writeline().)

I would personally also move your methods to below the rest of your code. That way, you have all of your "main" code in the same place, which makes it easier to follow. But this is just a personal preference and doesn't change anything about the behavior of your program.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Hate to be that pedantic guy, Dnomyar96 give a great addition, but the second line would be better using {op} for the operator rather than +: Console.WriteLine($"{a}{op}{b} = {c}"); \$\endgroup\$ Commented Nov 26 at 12:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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