Monday, August 24, 2009

One former coworker liked to add pointless parameter validation to methods. For example, a lookup method started like this

if (name == null || name.trim().length() == 0) {
log(some verbiage);
return null;
}

I'd prefer that code just not be there at all. If null is passed in and causes a NullPointerException, there's probably something wrong either with the code passing it in, and the exception would help in tracking down the problem more than the verbiage in the log statement would. There are situations where a null being passed in should be expected and should return a null, but that was not one of them.

The checking for an empty string or a string of only whitespace was really pointless, since it was a lookup that would have returned null anyhow for anything that wasn't in the table. Anyhow, the code calling the lookup method would either check the result and log something if it was null, or it would just use the result, causing a NullPointerException. Either way, the end result would be correct, though the NullPointerException made the logs uglier.

It wasn't as if the calls were part of an API to be delivered externally, or even to another internal team. And if they were, the checking would still have been mostly pointless.

No comments:

Post a Comment