Best way to check arguments before running method

Question

So, for example if I had a method called "divide" and it would just divide to ints and return an int, what would be the best way to check that the divisor is != 0

Way 1:

    private int divide(int i1, int i2) {
         if(i2 == 0) return 0;

         return i1/i2;
}

Way 2:

    private int divide(int i1, int i2) {
        if(i2 != 0) {
            return i1/i2;       
        } else {
            return 0;
        }
    }

Which one would you prefer?

EDIT: I know, that you'd normally not write such a function, since it would throw an exception if the divisor is 0 anyways. This is just to clearify my question.

EDIT 2: I just changed the method function to return 0, if the divisor is 0.

Answer 1

It is more common and (I believe more widely accepted) to use the first method, as if you are checking more than one or two variables, you begin to get very long line indentations. The first method is also more concise as compared to the second.

Answer 2

I prefer second one because its readable and if your requirement change in future like if i2==o then you may maintain it easily.

Answer 3

If your aim is to throw a RuntimeException whenever the second argument is 0, you do not need to do anything.

private int divide(int i1, int i2) {
    return i1/i2; // This will throw an ArithmeticException
}

The above code will throw an ArithmeticException which extends from RuntimeException thereby reducing your work of throwing a RuntimeException explicitly.

Edit: Since the code snippet in your question was just a sample, here is the updated answer.

It is more cleaner and better to avoid an unnecessary else such as in this case, because the else is another branch block and if you're really not going to do much in there than just a return, the code is better of without it.

If it goes into the if, it returns, thereby leaving the else useless and if it doesn't, then just the return is enough as all you need to do is return back from the method.

private int divide(int i1, int i2) {
    if(i2 == 0) {
        return 0;
    }
    return i1/i2;
}

Answer 4

I would prefer the second way, because it's more readable like Sanjay Rajjadi said,

private int divide(int i1, int i2) {
    if(i2 != 0) {
        return i1/i2;
    } else {
        throw new RuntimeException();
    }
}

but why throwing the exception? u can show an Alert or a message to the user asking him to insert a new int.

Answer 5

Personally, I prefer none of them, because throwing a RuntimeException is not a good practice; you should create your own subclass of Exception. Besides that, I can't see why it is necessary to explicitly throw this exception, as the resulting exception (java.lang.ArithmeticException) is a subclass of RuntimeException.
Maybe you should try something like this?

private int divide(int i1, int i2) throws DivisionByZeroException {
    try {
        return i1/i2;
    } catch(ArithmeticException e) {
        throw new DivisionByZeroException(e);
    }

Answer 6

private int divide(int i1, int i2) throws IllegalArgumentException{
    if(i2==0)
    throw new IllegalArgumentException("Argument 'divisor' is 0");
    else return i1/i2;
}

Answer 7

None.

You don't have to explicitly throw that exception, java.lang.ArithmeticException will be thrown if the divisor is zero.

Answer 8

If I really have to, I would use the first method because of the massive amount of indentations one will get if checking many arguements.

Nevertheless I have to say: Always evaluate twice if such a check is needed.

In your particular case, the check is redundant as division by 0 will throw an ArithmeticException whichs will say: / by zero.

See this question for a brilliant explanation why one often exaggerates with checks, especially with null/not null checks.

Answer 9

Use the below method. You need not to check for zero. Just throw arithmetic exception.

private int divide( int i1, int i2 ) throws ArithmeticException
{
    return i1/i2;
}