Just missed the train, wait for the next one to continue.

Don’t put view != null checks in your Presenters

When you’re using MPV on Android, especially when your presenter survives configuration changes, the presenter has at least two methods:

void attachView(View) and void detachView()

Because the View can be detached your View getView(); method should be marked with @Nullable. But now you have to throw if (view != null) checks everywhere or you’ll see tons of lint warnings of possible NullPointerExceptions. Even in cases where you can be 100% sure that the View is there.

Presenter method called directly from View

In this example onSettingsButtonClicked() gets called directly from the View. It’s not possible that the View is null.

How can we remove this lint warning?

This is the wrong questions! It’s not our job to remove lint warnings. They draw attention to possible mistakes. We should not remove the warning but solve the problem.

Most developers would guard the call with a if (view != null) check.

This removes the lint warning but what happens when there is a bug in theattachView(View) logic? The user presses the button and nothing happens. The feature is broken but nobody reports it because the error case was ignored.

It’s always a bad sign when the else branch is missing.

When you expect the view to be attached and it isn’t, it’s better to explicitly throw an Exception. If something goes wrong your app should crash so you are informed about this bad state via Crashlytics. (You should read about squares crash fast strategy). You might think this is bad for the user. Yes it is but most of those crashes are found during development and QA. And your beta testers are most likely ok with a crash.

It’s quite annoying to write this code over and over again. This can be simplified with a @NonNull View getViewOrThrow() method in your abstract presenter. The code is even shorter than guarding it with if (view != null).

• Lint is happy
• No missing else branch
• Your coworkers can’t misuse the method when the View is detached as it would be possible with the if (view != null) solution. A showErrorDialog() method should always show a dialog and not quietly do nothing when the view is not attached.

Fail fast, don’t swallow errors and run in unexpected states

Call View async from Presenter

Android is asynchronous and most calls to the View are asynchronous, too. Most commonly we show new data after a HTTP request returned. We can’t use getViewOrThrow(), it’s a valid case that the View is detached. getView() will result in a NullPointerException. Lint already shows a warning here.

Fixing the lint warning with a if (view != null) check at least removes the lint warning but it the wrong choice here. The else branch is very important. Without it the user may miss the server result and wait forever.

This is a possible implementation of the else branch. I must admit it’s quite complex for such a simple case.

Because it’s quite complex to write a working else branch, a dev under pressure will simply apply the if (view != null) quick fix. Bug resolved, the app doesn’t crash anymore. I’ve seen it multiple times and I’m sick of it.

A far better solution

At grandcentrix we develop and use the ThirtyInch MVP library. It has a unique method called sendToView(ViewAction) which delays the execution of a ViewAction (lambda) until the View is attached again. When the View is already attached it executes immediately. As simple as writing a if (view != null) check.

There is no additional code required in onAttachView(View). The postponed ViewActions are automatically executed after the View is attached.

grandcentrix/ThirtyInch

Just a warning to those who think sendToView might be the holy grail. Don’t overuse it! When you find yourself writing sendToView over and over again you should work on you architecture. Better maintain a ViewModel and render it to the View on change which also removes all if (view != null) checks in your application code.

Optionals and WeakReferences

This article uses view == null as synonym for the View being detached. When your MVP implementation is using wrappers like WeakReference or Optional for the View you obviously don’t need null checks. But this doesn’t mean you solved the problem, it is even harder because you have to use other checks and lint doesn’t help here:

Conclusion

You don’t need if (view != null) checks.

• Use getViewOrThrow() when you can be sure the View is attached to fail fast.
• Use sendToView(ViewAction) in cases the View can be detached to support the else branch.

When you really need a if (view != null) check write a proper else block which does the job or at least reports the bad state.

Don’t quick fix lint warnings, solve the problem for all cases.

Hit me on Twitter for feedback, tips and additional questions!

Pascal Welsch (@passsy) | Twitter