Trevor Elkins
Published on
/
5 min read

Eliminate Uncertainty In Your Code

Preface

I read two excellent blog posts that sparked my interest of the subject, found here and here.

Catch All the Exceptions!

Many programmers are afraid to let their apps crash. Your boss will find out, the whole team can see in the Crashlytics console, you'll look bad to your peers, the US economy will collapse... I think you get the picture. Unfortunately this kind of thinking leads to code that looks like:

public void blah() {
try {
someObject.doSomething();

        try {
            if (someObject.getSomething() != null) {
                anotherObject.doMore();
            }
        }
        catch (NullPointerException e) {
            e.printStackTrace();
        }
    }
    catch (Exception e) {
        e.printStackTrace();
    }

}

This code is in rough shape.

There's a tipping point where overly defensive code becomes harmful. Not every exception or error can be recovered from.

Only try to recover from an error if you know you can, otherwise you're putting your program in a weird state. Code that runs afterwards will need to be more robust; if it's not then you're deferring the crash to another section of your code. This is a bigger problem—the real issue will be masked by a different stacktrace!

What to Check?

I will echo the sentiments of this post.

  • Is this exceptional?
  • Is this expected?
  • Can I continue doing something meaningful at this point?
  • Is this something I should be made aware of so I can fix it, or at least give it a second look?

This is sage advice. Only check an exception if it's exceptional and you plan to do something with it. I weep when I see:

try {
someService.dangerousMethod();
} catch(Exception e) {
e.printStackTrace();
}

This isn't doing anything meaningful. I'd guess that this exception was only handled to make the compiler stop complaining. At the very least you should be forwarding the Exception to a reporting service. I just recently found out Crashlytics supports this.

I know this is a religious subject to some so I'll be brief, but I'm generally against checking exceptions. This sums up most of my feelings. In practice I've found exceptional cases to handle very rare.

Over-checking Null

Bad design will trickle down to the lower, more concrete, levels of your code. For some reason, lots of programmers are happy dealing with the problem at the bottom when a small refactor could solve the problem at the top. Why put a bucket under a drip when you can fix the leak?

The biggest culprit I see is propagating null through your code. I try to never pass null to another method since it shouldn't have to worry about an argument existing. For example, this is bad:

public void someMethod() {
Blah blah = getBlah(); //blah could be null
otherMethod(blah);
otherMethod2(blah);
}

public void otherMethod(Blah blah) {
if (blah != null) {
// do something
}
}

public void otherMethod2(Blah blah) {
if (blah == null) {
return;
}
// do something
}

That looks silly. Instead, refactor it to:

public void someMethod() {
Blah blah = getBlah();
if (blah != null) {
otherMethod(blah); //don't propagate the null!
}
}

public void otherMethod(Blah blah) {
// do something
}

Branches (if statements) make code much harder to understand. Code that's hard to understand is tough to make correct. You're doing yourself a favor by having less null checks.

There's many ways to eliminate nulls from your code, for instance, you can return an empty collection if no results are found or even a default object. You can take advantage of the @NonNull and @Nullable annotations that were added recently.

What this boils down to is being able to make assumptions and guarantees. Code should be able to make assumptions based on what ran before it, otherwise it looks like you're uncertain whether something will behave like you expect it to.

Eliminating Uncertainty

So far I've talked about when to check exceptions and how you should reduce the amount of nulls in your code, but this is what I want you to take away from all this: don't allow behavior you don't want.

Here's a basic example:

public void onClick(View view) {
switch(view.getId()) {
case R.id.someButton:
break;
case R.id.anotherButton:
break;
default:
throw new IllegalArgumentException("Unsupported id " + view.getId());
}
}

This method is perfectly correct without the default case and would chug happily along if it received a View ID it didn't recognize. We don't want that, we want to fail hard and fast in that case. The exception is for our benefit.

Your code will grow over time, and sometimes the tools at your disposal aren't enough; you have to help yourself. There's no compilation error or Lint warning in my example.

Now, obviously a developer would notice if their button didn't work, but think about this on a larger scale. Will you really remember all the moving parts your change will affect? Tests help catch most things, and this helps too.

Preconditions

After coming across this in Google's Guava project, I've been using this to great success:

public class SomeObject {

    private OtherObject otherObject;

    public SomeObject(OtherObject otherObject) {
        this.otherObject = Preconditions.checkNotNull(otherObject);
    }

}

Where Preconditions.checkNotNull() throws an exception if its argument is null. This style is nice because you can assign and check in one line.

Discuss on Twitter