Java Validation – Should Method Call Return Values Be Validated Even if Safe?

defensive-programmingjavanullreturn-typevalidation

I'm wondering if I should defend against a method call's return value by validating that they meet my expectations even if I know that the method I'm calling will meet such expectations.

GIVEN

User getUser(Int id)
{
    User temp = new User(id);
    temp.setName("John");

    return temp;
}

SHOULD I DO

void myMethod()
{
    User user = getUser(1234);
    System.out.println(user.getName());
}

OR

void myMethod()
{
    User user = getUser(1234);
    
    // Validating
    Preconditions.checkNotNull(user, "User can not be null.");
    Preconditions.checkNotNull(user.getName(), "User's name can not be null.");

    System.out.println(user.getName());
}

I'm asking this at the conceptual level. If I know the inner workings of the method I'm calling. Either because I wrote it or I inspected it. And the logic of the possible values it returns meet my preconditions. Is it "better" or "more appropriate" to skip the validation, or should I still defend against wrong values before proceeding with the method I'm currently implementing even if it should always pass.

My conclusion from all answers (feel free to come to your own):

Assert when

  • The method has shown to misbehave in the past
  • The method is from an untrusted source
  • The method is used from other places, and does not explicitly state it's post-conditions

Do not assert when:

  • The method lives closely to yours (see chosen answer for details)
  • The method explicitly defines it's contract with something like proper doc, type safety, a unit test, or a post-condition check
  • Performance is critical (in which case, a debug mode assert could work as a hybrid approach)

Best Answer

That depends on how likely getUser and myMethod are to change, and more importantly, how likely they are to change independently of each other.

If you somehow know for certain that getUser will never, ever, ever change in the future, then yes it's a waste of time validating it, as much as it is to waste time validating that i has a value of 3 immediately after an i = 3; statement. In reality, you don't know that. But there are some things you do know:

  • Do these two functions "live together"? In other words, do they have the same people maintaining them, are they part of the same file, and thus are they likely to stay "in sync" with each other on their own? In this case it's probably overkill to add validation checks, since that merely means more code that has to change (and potentially spawn bugs) every time the two functions change or get refactored into a different number of functions.

  • Is the getUser function is part of a documented API with a specific contract, and myMethod merely a client of said API in another codebase? If so, you can read that documentation to find out whether you should be validating return values (or pre-validating input parameters!) or if it really is safe to blindly follow the happy path. If the documentation does not make this clear, ask the maintainer to fix that.

  • Finally, if this particular function has suddenly and unexpectedly changed its behavior in the past, in a way that broke your code, you have every right to be paranoid about it. Bugs tend to cluster.

Note that all of the above applies even if you are the original author of both functions. We don't know if these two functions are expected to "live together" for the rest of their lives, or if they'll slowly drift apart into separate modules, or if you have somehow shot yourself in the foot with a bug in older versions of getUser. But you can probably make a pretty decent guess.

Related Topic