5
\$\begingroup\$

I am new to coding, I hope you can help me to improve my code :)

First of all: The code works correctly.

using System;
namespace _3_dividing_2numbers
{
    class Program
    {
        static void Main(string[] args)
        {
            
            try
            {
                OperationDevide();
            }
            catch (DivideByZeroException ex)
            {
                Console.WriteLine(ex.Message);
                Console.WriteLine("Enter another number !");
                OperationDevide();
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex.Message);
                OperationDevide();
            }
        }
        public static void OperationDevide()
        {
            Console.WriteLine("Enter your first number :");
            var firstNum = Convert.ToInt32(Console.ReadLine());
            Console.WriteLine("Enter your second number :");
            var secondNum = Convert.ToInt32(Console.ReadLine());
            var result = firstNum / secondNum;
            Console.WriteLine($"your result is : {result}");
        }
    }
}

Code description

I write a simple program that takes two int inputs and prints the result. but I want to check the possible errors so I used from try/catch for DivideByZeroException and all of the other errors by Exception.

My Problem

Do you have any idea to make this code better? I want to keep asking the user until the inputs have the correct format. when I run my code after the second Zero input it's crashed and try/catch only works for the first round of Zero inputs.

\$\endgroup\$
8
  • 3
    \$\begingroup\$ Micro-review - spelling fix gives OperationDivide. \$\endgroup\$ Feb 13 at 16:59
  • 2
    \$\begingroup\$ Please do not edit the question, especially the code, after an answer has been posted. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered. \$\endgroup\$
    – pacmaninbw
    Feb 13 at 17:11
  • 3
    \$\begingroup\$ Does it have to be an exception? Usually we just check if the second number is 0, before dividing \$\endgroup\$
    – user253751
    Feb 13 at 20:53
  • 2
    \$\begingroup\$ Throwing an exception is more expensive than a quick early check that the divisor is not 0. \$\endgroup\$
    – Rick Davin
    Feb 13 at 21:02
  • 4
    \$\begingroup\$ @RickDavin In some languages / implementations try / catch has zero cost or practically zero cost if no exception is thrown. And if it is thrown, you don't care about reasonable cost. \$\endgroup\$
    – gnasher729
    Feb 14 at 12:22

4 Answers 4

9
\$\begingroup\$

If you need a repetition either use a recursive call or a loop. In this situation a loop makes sense.

while (true) { // Loop forever or until e.g., return or break exits the loop.
    try
    {
        OperationDevide();
        break; // The operation succeeded. Exit the loop.
    }
    catch (DivideByZeroException ex)
    {
        Console.WriteLine(ex.Message);
        Console.WriteLine("Enter another number !");
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.Message);
        Console.WriteLine("Try again !");
    }
}

Note that when an exception occurs, the control flow is redirected to a catch-statement and the break statement is not executed.


As others have mentioned, exceptions can be avoided here. Not only the division by zero but also parsing errors. Let's try another approach.

I refactor the division method to contain only logic and math, but no input or output operations. The aim is to a have a separation of concerns: the Main method does input and output. The other method does business logic, as this is often called.

// Returns null if the divisor was 0 and the integer quotient otherwise
public static int? Divide(int dividend, int divisor)
{
    if (divisor == 0) {
        return null;
    }
    return dividend / divisor;
}

Let's use Int32.TryParse instead of Convert.ToInt32. The former returns a Boolean value return value indicating whether the operation succeeded and does not throw exceptions. The latter throws an exception if the string cannot be converted to an int. Also, since we have to input numbers repeatedly, we can also refactor out this procedure to yet another method. This also makes the main method clearer.

private static int ReadIntegerInput(string name)
{
    while (true) {
        Console.Write($"Enter the {name}: ");
        string input = Console.ReadLine();
        if (Int32.TryParse(input, out int number)) {
            return number;
        }
        Console.WriteLine($"Your {name} input is not a valid integer. Please try again.");
    }
}

An advantage is that we do not need to repeat entering both numbers if something goes wrong.

The body of the main method can the be written as:

int dividend = ReadIntegerInput("dividend");

while (true) {
    int divisor = ReadIntegerInput("divisor");

    int? result = Divide(dividend, divisor);
    if (result == null) {
        Console.WriteLine("The divisor cannot be zero. Please try another divisor.");
    } else {
        Console.WriteLine($"The integer quotient of {dividend}/{divisor} is : {result}");
        break;
    }
}
\$\endgroup\$
4
  • \$\begingroup\$ Thank you so much. so can we use from Environment.Exit(0); in the catch section to go exit from the loop? and I try to use do-while but I wasn't sure to what bool expression should use in the while () part. \$\endgroup\$
    – jasmine
    Feb 13 at 16:56
  • 1
    \$\begingroup\$ This would work as well, but in the Main method you can also call return; to exit the application. You can also declare it as public static int Main() and then call return 0;. See Main() and command-line arguments. \$\endgroup\$ Feb 13 at 17:02
  • 1
    \$\begingroup\$ The problem with returning null is that you've hidden the fact that the method can do something inappropriate, and instead of DivisionByZeroException (the cause is obvious) you'll find NullReferenceException popping unexpectedly out of other areas of the code (which means firing up a debugger) \$\endgroup\$ Feb 15 at 11:03
  • 1
    \$\begingroup\$ Floating point division can lead a an overflow (big number divided by small number) but not integer division. Division by zero is in fact the only bad thing which can happen. \$\endgroup\$ Feb 15 at 12:48
12
\$\begingroup\$

If you know that you want to divide two numbers then IMHO it would make more sense to perform a pre-liminary check.

In other words, rather than executing the operation "blindly" by trusting the user provided data, you could simply check that the pre-conditions are met. In this particular case the divider is a number and not zero

int divider = 0;

do
{
   Console.WriteLine("Please enter a number other than zero:");
   var isNumber = int.TryParse(Console.ReadLine(), out divider);
   if (!isNumber || divider == 0)
       Console.WriteLine("Provided data can't be used. Please try again");

} while (divider == 0);
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Thanks. I was searching for ways to handle the errors and I read an article that says that most of the time we use TryParse more than Try/Catch. so this way is more common and better for my problem. As you mentioned if we know what input and conditions are we should use TryParse to check the conditions are true or not and if we don't sure about the possible errors or input we use Try/Catch \$\endgroup\$
    – jasmine
    Feb 14 at 7:51
  • 3
    \$\begingroup\$ @jasmine Yes, you can look the problem like that. As a rule of thumb if you try to write defensive code (not paranoid) then you can minimise the assumptions. And that would shift your error-handling from mitigation to prevention. \$\endgroup\$ Feb 14 at 8:15
  • 2
    \$\begingroup\$ TryParse just saves you a few lines of code and may be easier to implement. if TryParse { success case } else { failure case } is often easier to use. What happens is the same. Especially if you only care whether your operation failed or succeeded, not why it failed. \$\endgroup\$
    – gnasher729
    Feb 14 at 12:32
  • 1
    \$\begingroup\$ @gnasher729 I've just wanted to emphasize that we should perform pre-liminary checks that's why I added || divider == 0, but yes you are right this could rewritten like you described. \$\endgroup\$ Feb 14 at 12:39
3
\$\begingroup\$

The exception is really not the problem. The problem is: What should you do to help the user achieve what they want to achieve?

If the user can enter data that lead to this error, then you would probably want to tell the user which data was incorrect, so they can either fix the data and you try again, or they cancel whatever they were doing.

And there are situations where you can fix the problem. Replacing 0/0 with a zero might give a correct result. That's for you to decide or implement.

On the other hand, you might have done checks so you think "at this point, it is impossible that we divide by zero". Yet it happens. Now you really need to figure out: What should you do in a situation that you thought was impossible, yet it happened? If your logic is used to decide that a self driving car should accelerate to x/y miles per hour, and say x = 3, y = 0, a subject matter expert should decide what should happen, because the wrong decision could cost lives.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you so much. now I can write better code with these explanations. I think I didn't clear the problem very well. \$\endgroup\$
    – jasmine
    Feb 14 at 16:31
  • 1
    \$\begingroup\$ @jasmine It might be helpful to think about a concrete use case, such as writing code for an ATM. Choosing the account, and whether to print a receipt might be conditional statements; not knowing the PIN or wrong type of card might be exceptions. \$\endgroup\$ Feb 15 at 18:56
2
\$\begingroup\$

Code should tell a story. If you are writing code commercially, some person will need one day to fix defects or add new functionality, so they will first have to read and understand it (that person may even be your good self). Code can either reveal the developer's intentions, or hide them.

What does an exception tell the reader, compared to a conditional statement? An exception says that the author considers a situation to be something wrong, something exceptional, something to be recovered from. A conditional tells the reader that the conditional is part of the normal flow. So ask yourself: what are you trying to say, and have you said it well, from a human point of view.

\$\endgroup\$
2
  • \$\begingroup\$ So after writing the code, we should spend time on the requirements that this solution fulfilled for the user, understanding the problem fulfilly. Thanks 🙏 \$\endgroup\$
    – jasmine
    Feb 15 at 4:27
  • 1
    \$\begingroup\$ @jasmine Before writing the code might be better. \$\endgroup\$ Feb 15 at 18:46

Your Answer

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

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