Is it better in Java to make all necessary checks from the caller or in the method it calls?

Question

Let's look at these two examples.

First:

try {
    execute(testObj);
} catch(Exception e) {
    //do somethingwith that
}

public void execute(TestObj testObj) throws Exception {
    if (testObj == null){
        throw new Exception("No such object");
    }
    //do something with object
}

Second:

if (testObj != null){
    execute(testObj);
} else {
    //handle this differently
}

public void execute(TestObj testObj) {
    //do something with object
}

It is not the point right now if we need to check "is null" or anything else. I want to know which practice is better in overall – "check, then do" or "do, then handle exception if occurs"?

Answer 1

Neither, you should only do checking on the boundaries between systems.

What is boundaries between system?

1. When you receive input from the user,
2. When you read a file, network, or socket,
3. When you do system calls or interprocess communication,
4. In library codes, in all public-facing APIs.
5. etc.

In library code, since public APIs may be called by external code, you should always do check in anything that can be called by external code. There is no need to do checking for an internal-only method (even if its access modifier is public). Argument errors may then be signalled by exception or by return code depending on personal taste, however checked exception cannot be ignored so it's usually the preferred method to signal errors to external codes.

In an application code (as opposed to a library), checking should only be done when receiving input from user, when you load a URL, when reading file, etc. There is no need to do input check in public methods because they are only ever called by your own application code.

Within your own code, you should avoid the situation where you need to check in the first place. For example, instead of checking for null, you can use "null object pattern". If you still need to do a check though, do it as early as possible, this usually means doing it outside but try to find even earlier points.

Even if not written down formally and even if it's not enforced, all methods whether its internal or public-facing should have a contract. The difference is that in external-facing code, you should enforce the contract as part of your error checking, while in internal-only code you can and should rely on the caller knowing what to and not to pass.

In short, since checking is only done on system boundaries, if you actually have a choice of whether to check inside or outside a method, then you probably should not be checking there since that is not a system boundary.

In a system boundary, both sides always have to check. The caller had to check that the external call succeeds and the callee had to check that the caller gives a sane input/argument. Ignoring an error in the caller is almost always a bug. Robustness principle applies, always check everything you receive from external system, but only send what you know that the external system can accept for sure.

In very big projects though, it's often necessary to compartmentalize the project into small modules. In this case, the other modules in the project can be treated as external systems and you should therefore do the checks in the APIs that are exposed to the other modules in the project.

TLDR; checking is hard.

Answer 2

As @LieRyan said, you should only do checking on the boundaries between systems. But once you are inside your own code, you still need to be able to detect unexpected problems, mostly because:

• you (and your co workers) are not perfect and may pass null to a method that can not handle it.
• if one line uses two objects and one is null the NPE will not tell you which one is to blame (System.out.println ("a.getName() " + "b.getName()") throws NPE...is a null or b null or both?)

Clearly document which methods accept null as parameters, or may return null.
A nice tool that can help you, if you are using Eclipse Juno, (I think IntelliJ-Idea has something similar) is to enable null checking analysis. It allow you to write some annotations to make compile time null checking. It is really awesome. You can write something like

public @NonNull String method(@Nullable String a){
//since a may be null, you need to make the check or code will not compile
    if(a != null){
        return a.toUppercase();        
    }
    return ""; //if you write return null it won't compile, 
     //because method is marked NonNull 
}

It also has a nice @NonNullByDefault, which basically says "No method accepts or returns null unless marked @Nullable" This is a nice default, keeps your code clean and safe.

For more info, check Eclipse help

Answer 3

I would say it depends on the type of exception. If the exception is down to invalid arguments, check them in the method. Otherwise, leave it to the caller to handle.

Example:

public void doSomething(MyObject arg1, MyObject arg2) throw SomeException {
   if (arg1==null || arg2==null) {
       throw new IllegalArgumentException(); //unchecked
   }
   // do something which could throw SomeException
   return;
}

calling:

try {
    doSomething(arg1, arg2);
} catch (SomeException e) {
    //handle
}

Answer 4

It depends. Both approaches are good enough. First is good when your function will be used by other clients. In this case your checks prevent failures because of illegal arguments. As author you guarantee that your code works fine even in case of unexpected bad input arguments. The second can be used when you are not an author of the function or don't want to handle exceptions thrown by the method.

Answer 5

According to Joshua Bloch Effective Java Second Edition, item 38, it is better way to check parameters for validity inside public methods. So, the first approach is much better, except the fact, that in this situation you have to throw basically NullPointerException, which is RuntimeException:

// do not need try...catch due to RuntimeException
execute(testObj);

public void execute(TestObj testObj) {
    if (testObj == null){
        throw new NullPointerException("No such object");
    }
    ;//do smth with object
}

Answer 6

The second is better as in the first, any exception could fire and you would think this is expected.

Bare in mind also using exceptions to control program flow is computationally expensive as building the exception costs performance. For instance, in hibernate, it's better to do a SELECT COUNT(*) to check if a row exists than seeing if a recordNotFoundException fires and basing your application logic on that exception firing.

Answer 7

the second approach is better, because it is easier to read and maintain.

You should also keep in mind that throwing exceptions sometimes only moves your problem from A to B and it does not solve the problem.

Answer 8

It's better to do it from the caller because you can, for example if the data is given from the user, request new data if they are not suitable or not call the function if the data is not good. Therefor you can keep your method "clean" and that certain method will only perform it's simple purpose function. Therefor it will be more easily transferable and reusable in other areas. You can, if the data provided to the method to throw an exception.

E.G. if it is a method that writes data to a file and and it gets the file path as input. You can throw a FileNotFound exception.

In that way the programmer using your method will know that he must provide a proper file path and do the all the necessary checks before hand.

Answer 9

By purpose:

If you are creating a library, you are just providing APIs rather than some runnable code. You don't know whether users will make the check from caller, so you can (not necessarily, sometimes if statement does the job) throw exceptions from your method to ensure the robustness.

If you are creating an application, the second way is much better. It's a bad practice to use try-catch clause only for check of simple conditions.

In fact, exception throwing statements and if-checks could co-exist and I recommend it. But in your code, you should avoid to cause exceptions as any as you could.

---

By usage:

Exceptions are thrown when some situations are not expected but could happen. Normally it is useful for debugging purpose because you can trace the stack by evaluating the Exception instance.

If-clause is useful in all cases. In fact, throw statements are often wrapped in if clause. It means you don't want to (or can't) deal with that condition, you just leave it to the debugger (exception manager). Or instead you can write your code as you like.

---

At most times, you would like to deal with all the conditions, although some are less possible to happen. As I said, try to throw as few exceptions as possible, not only for performance concerns, also for convenience of use.

A (bad) example to show that sometimes exceptions are wasteful:

public void method1 () throws Exception1, Exception2 {
    if (condition) {
        throw new Exception1("condition match");
    } else {
        throw new Exception2("condition mismatch");
    }
}

public void method2 () {
    try {
        method1();
    } catch (Exception1 e1) {
        // do something
    } catch (Exception2 e2) {
        // do something
    }
}

Normally, you won't write code like this, but as you can see, one method is splited into two and method1 almost does nothing. It's the same with your first example, never split the code which belongs to one method in different places.

Answer 10

Exceptions should be used for exceptional conditions; things you don't expect to happen. Validating input isn't very exceptional.

Answer 11

One should endeavor, whenever possible, to ensure that if an exception is thrown from a method, the failed operation will have had no side-effects. While there is alas no standard means in Java or .net via which which an exception can indicate that it represents a "clean side-effect free failure", there are many scenarios (e.g. trying to deserialize a data structure) where it's hard to predict all the types of failures may occur (e.g. as a result of trying to load a corrupted file) but where nearly all types of side-effect-free failure may be handled the same way (e.g. inform the user that a document could not be loaded, most likely because it's either corrupt or in the wrong format).

Even if a method will be used only within one's own code, it is often good to validate parameters before doing anything with them which could adversely affect other code. For example, suppose code uses a lazily-sorted list (i.e. it keeps a list that may or may not be sorted, along with a flag which indicates whether it's sorted or not); when adding an item to the list, it simply appends it to the end and sets the "sorting required" flag. The "add item" routine might complete "successfully" if it was given a null reference or an object which couldn't be compared to items in the list, but adding such an item would cause later sort operations to fail. Even if the "add item" method wouldn't have to care about whether the item was valid, it would be much better for the "add item" method to throw an exception, than to leave the list in a state where later operations might fail (at that point, even if one found out that the sort failed because of a null item in the list, one might have no way to determine how it got there).

Then trouble-shooting code, if method foo receives a parameter value which will be immediately passed to bar and cause bar to throw, having the exception thrown by foo rather than bar may be somewhat helpful, but the scenario where the exception will end up getting thrown essentially immediately and without side-effects--even if by a nested method--is far less important than the scenarios where foo would have cause some side-effects before it thrown an exception or--even worse--complete "normally" but cause future operations to fail.