Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

when to throw exception if illegal argument passed to an API method?

See the two examples below. Assume both classes contains public methods belonging to a widely used API library.

AClass.java is easier to write (maybe easier to read due to less noise), but when error is detected, the first method in stack trace is an internal method (nullCheck(...)) and the exception message is not referring to that method. Could this cause unnecessary confusion to the user? I mean in the sense of the user thinking: "it's an internal method which has thrown an exception, must be a bug in the library, not a fault in my program. What a ** useless library..."

BClass.java is more tiresome to write (with all its if statements), but when error is detected the first line of the stack trace pinpoints the API method (called by user) where error was first detected. Would this more likely make user think: "I'm calling that method from my code, must be something wrong with the parameter I'm passing in".

Which would be the preferred way of throwing exception when checking parameters for validity in a public API? Or are the two examples considered equal?

public class AClass {
public void publicApiMethod1(String a){
    nullCheck(a, "a");
    // do something...
}
public void publicApiMethod2(String a, String b, String c){
    nullCheck(a, "a");
    nullCheck(b, "b");
    nullCheck(c, "c");
    // do something...
}

private void nullCheck(Object a, String argName) {
    if(a == null){
        throw new NullPointerException("Null argument: " + argName);
    }
}
}


public class BClass {
public void publicApiMethod1(String a){
    if(a == null){
        throw new NullPointerException("Null argument: 'a'");
    }
    // do something...
}

public void publicApiMethod2(String a, String b, String c){
    if(a == null){
        throw new NullPointerException("Null argument: 'a'");
    }
    if(b == null){
        throw new NullPointerException("Null argument: 'b'");
    }
    if(c == null){
        throw new NullPointerException("Null argument: 'c'");
    }
    // do something...
}
}
like image 547
etxalpo Avatar asked Jan 16 '23 13:01

etxalpo


1 Answers

If your error message is descriptive (and it is), no one will bother to look at the stack trace. Thus the first form is better because it encapsulates the validation logic.

Note that there are plenty of assertion methods in various libraries ready to use, see: Objects.requireNonNull and Validate.notNull).

like image 185
Tomasz Nurkiewicz Avatar answered Jan 20 '23 15:01

Tomasz Nurkiewicz