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
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
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 the
attachView(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
elsebranch 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
- Your coworkers can’t misuse the method when the
Viewis 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.
ThirtyInch - a MVP library for Android favoring a stateful Presenter
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
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:
You don’t need
if (view != null) checks.
getViewOrThrow()when you can be sure the
Viewis attached to fail fast.
sendToView(ViewAction)in cases the
Viewcan be detached to support the
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!