A Rule of Thumb For Controller Code

Bill Phillips's Headshot
Bill Phillips

(Special guest posting by Bill Phillips, Android Bootcamp instructor, coauthor of the forthcoming book Android Programming: The Big Nerd Ranch Guide, and Clash of the Coders winner.)

When you start writing a Controller, write one method that sets up your entire user interface to reflect the current state of your model objects. When any single portion of the model changes, call that method to update the entire interface.

Why?

The Scenario

Let's say you need to write an Android activity that displays information about a television show for a DVR interface. You create a new activity, and your onCreate() method looks something like this:

public void onCreate() {
    // setup the initial user interface
}

Next, you look at your requirements list. It says, "When the show is recorded, the 'record show' button should be changed to the 'delete recording' button." "Ahh," you say. "I can do this." You translate that directly into the following code:

public void recordShow() {
    show.setRecorded(true);
    showButton.setImage(deleteRecordingImage);
}

You test it, it works. Fantastic. You put your shades on and strike a pose.

Next week, you get a bug report in from your offshore collaborators: after some back and forth, it turns out that when QA navigated to your activity with a show that was already recorded, the user interface showed that the show wasn't recorded. "Simple enough," you say, and you add the following code:

public void updateShowButton() {
    if (show.isRecorded()) {
        showButton.setImage(deleteRecordingImage);
    } else {
        showButton.setImage(recordImage);
    }
}

public void recordShow() {
    show.setRecorded(true);
    updateShowButton();
}

public void onCreate(Bundle bizzundle) {
    // setup the initial user interface
    updateShowButton();
}

You're no slouch as a coder, so you pull the showButton setup code into its own method while you're at it rather than repeating yourself in onCreate().

It tests out, it works. You put your shades on and open your eyes extremely wide behind them like Nicholas Cage probably does at home alone.

The next week after that, you get a call from the designer while you're jamming out to jazz fusion. "Fredward," he says, "Didn't you see in the wireframes that the 'Move Show To Hugbox' button needs to be in the television show activity?" After some back and forth, you determine that the designer forgot to include that button here like he did everywhere else in the interface. You add an issue to BugHole, your issue tracker, and get to work. You add the button, and the following code to update it appropriately:

public void updateHugboxButton() {
    if (show.isInHugbox()) {
        hugboxButton.setEnabled(false);
    } else {
        hugboxButton.setEnabled(true);
    }
}

public void onHugboxButtonClicked() {
    show.moveToHugbox();
    updateHugboxButton();
}

For the sake of consistency, you decide to follow the same pattern you ended up using for the showButton update code.

You test it, it works. You put your shades on, but find yourself strangely apprehensive for some reason.

Of course, the next week, you get another bug report: QA says that the television activity is setup improperly again when you first pull it up - shows that are in the hugbox already have an enabled hugbox button. You made the exact same slip as before, and the UI is in an inconsistent state. In shame, you throw your shades away and go home with a six pack of Mike's Hard Lemonade and a Redbox DVD of "Stripes".

The Lesson

So. To revisit the point from the beginning of this little article:

When you start writing a Controller, write one method that sets up your entire user interface to reflect the current state of your model objects. When any single portion of the model changes, call that method to update the entire interface.

This short example is uncomplicated compared to many real-world controllers. You may easily have three or four times as many things going on as this code. The show's entire series may be recorded, the recording button might need to be disabled because the show is already over, other aspects of the look and feel of the interface may need to change for a thousand reasons. As the interface gets more complicated, the sorts of issues seen here only get worse.

One way to deal with making mistakes like this is to beat yourself up for making the mistake, and swear: "I promise that I will be more careful in the future! I will remember to call my update code in all the appropriate places!" Other people may want you to flog yourself in this way, but unfortunately it yields poor results. The better way to deal with it is to change your methods so that the bug is impossible.

In the example just reviewed here, update was handled on an ad hoc basis. When we recorded a show, we said "Change the record button to a delete button." When we moved a show to the hugbox, we said, "Turn the hugbox button off."

The alternative strategy is to instead do this: when we record a show, say "Update the user interface to reflect the new model state." When we move a show to the hugbox, we say, "Update the user interface to reflect the new model state." And when we first fire up our activity, we say the same thing.

The Fixup

Let's see how this strategy would have eliminated our problems. Here's how we would have initially written our onCreate():

public void updateUI() {
    // setup default view appearance
}

public void onCreate(Bundle bizzundle) {
    // boilerplate
    updateUI();
}

Next, we'd implement our first behavior: the recording button image change. Instead of updating only the button within the recordShow() method, we would write our implementation like so:

public void updateUI() {
    // setup default view appearance
    if (show.isRecorded()) {
        showButton.setImage(deleteRecordingImage);
    } else {
        showButton.setImage(recordImage);
    }
}

public void onCreate(Bundle bizzundle) {
    // boilerplate
    updateUI();
}

public void recordShow() {
    show.setRecorded(true);
    updateUI();
}

"Wait a second," you might say. "Aren't we doing our initial setup every single time a button is pressed here?" Yes, we are. Is it a performance problem? Perhaps. You're usually better off assuming "no". You can always measure later. This allows you to write the simplest thing that could possibly work.

Next, the hug box button directive comes up. You implement it this way, putting the user interface updates in one place, and the button handling elsewhere:

public void updateUI() {
    // check the state of the model to see what the button should look like
    if (show.isRecorded()) {
        showButton.setImage(deleteRecordingImage);
    } else {
        showButton.setImage(recordImage);
    }

    if (show.isInHugBox()) {
        hugboxButton.setEnabled(false);
    } else {
        hugboxButton.setEnabled(true);
    }
}

public void onHugboxButtonClicked() {
    show.moveToHugbox();
    updateUI();
}

Since updateUI() is already being called in other places where the model state transitions, all model state transitions get the updated behavior. You never have an opportunity to make the mistake of leaving the update code out of onCreate().

That's not the only kind of error this eliminates. Changes in model behavior can cause bugs, too. Let's say the designer decided that when shows are moved to the hugbox, they should also automatically be recorded:

public void onHugboxButtonClicked() {
    show.moveToHugbox();   // also calls setRecorded(true) now
    updateHugboxButton(); 
    // bug here - show is recorded, but showButton might still show recordImage!
}

The old code assumed that the recording state would only change when the recording button was pressed. As a result, changing the model behavior led to an inconsistent state in the user interface.

The reason this method is more robust is that the question, "What should my user interface look like for a given model state?" is answered in exactly one place. Each time you add some interaction to the interface, the answer to that question changes. If you litter those updates throughout your controller, you will have to piece that answer together, which could be confusing. And if you're confused, your code will be confused, too. And then your interface is confused, which confuses your user. And possibly their pets.

Of course, we never set out trying to write confusing code. (Well, at least most of us.) The requirement was simple, and we translated it into an implementation in a straightforward fashion. So consider the scenario outlined above a reminder: a straightforward implementation of a requirement doesn't always make for a straightforward controller.

(Think the Big Nerd Ranch is only about iOS and Cocoa? Think again! We do a lot of Android App Development. Bill also teaches the totally awesome, totally nerdy Android Bootcamp at the end of August. Space available!)

Recent Comments

comments powered by Disqus