There's actually a reason I put VBM in the title of this series. And aside from some lightweight organization of the UI layer, a major aspect to the VBM structure is utilizing the mediator as the layer to encapsulate any logic branching code and protect the rest of the system from such corrupting code; as much as possible anyway.

The primary goal of this post is to see how to make some decisions in the Mediator layer based on the network response; which means I'll get to fake some bad data!

Data Load

I was looking at how to pop something up when a story doesn't load then realized that it's an extra layer; and there will need to be some kinda debouncing if there's a network issue to prevent LOTS of "data error" stuff happening.
Doing the initial story request allows a simpler interaction set to start with. I just need to handle updating the stories or display a dialog on error.

Currently using my favorite design-pattern hammer; the strategy pattern!

/* package */ static abstract class DataLoadStrategy{
        public final static DataLoadStrategy DataChanged = new DataLoadStrategy(){
            @Override
            public void run(final TopStoriesActivity activity) {
                activity.notifyTopStoriesChanged();
            }
        };

        public final static DataLoadStrategy DataError = new DataLoadStrategy() {
            @Override
            public void run(TopStoriesActivity activity) {
                new AlertDialog.Builder(activity)
                        .setTitle("OMG, DIALOG")
                        .setMessage("Exciting").show();
            }
        };

        private DataLoadStrategy(){}
        public abstract void run(final TopStoriesActivity activity);
    }

The update to the bridge is super simple;
Going From

    /* package */ void notifyTopStoriesChanged(){
        topStoriesActivity.notifyTopStoriesChanged();
    }

To

    /* package */ void notifyTopStoriesChanged(final DataLoadStrategy dataLoadStrategy){
        dataLoadStrategy.run(topStoriesActivity);
    }

and that forces the change to the Mediator.

Now we're at the point the mediator requires some selection logic.
We don't REALLY need the selection logic extracted. I'm extracting it to help demonstrate the practice... kinda the whole point of all this. :)

    private TopStoriesActivityBridge.DataLoadStrategy dataLoadStrategyFactory(final Response<Stories> response){
        return response == null || !response.isSuccessful()
                ? TopStoriesActivityBridge.DataLoadStrategy.DataError
                : TopStoriesActivityBridge.DataLoadStrategy.DataChanged;
    }

This is a "for the future" bit of code - but once more logic is required; this extraction will be required; tossing it into a 'factory' now for clarity.

What I love about this - No tests broke.
What I hate about this - No tests broke.

I changed the API for the TopStoriesActivityBridge#notifyTopStoriesChanged and all tests passed. This implies there's no tests for that method.

Which; checking the tests at TopStoriesActivityBridgeTests, notifyTopStoriesChanged does not, in fact, have a test on it. :\

Other tests cover that flow; but it's not covered explicitly. Ruh-roh.
This is one where I'm not actually too worried about it; as it's getting covered from some other paths; but I'd prefer to have a test around it; as we have around the single story change method.

What I'm going to do; as can be seen above; is use an AlertDialog to indicate an error loading the collection of stories.
If I just write a test to hit that path... it explodes.

java.lang.RuntimeException: Method setTitle in android.app.AlertDialog$Builder not mocked. See http://g.co/androidstudio/not-mocked for details.

Writing Another Post

I took a bit of a tangent with this and wrote up a separate post for Dialog Testing.
What I did there would fit in right here very nicely... since it's chronologically where I did the work. I want to have the Dialog handling as it's own referable piece; so go refer to it.

The improvement on how to use the AlertDialog allows us to unit test all of our existing flow.

I was looking at how the Strategy pattern was implemented; and it's doing a selection; then passed into the Bridge to be executed... What if the Mediator actually calls run? It's one less hop to make; feels 'better'.
This starts a process of removing the calls into the Bridge. Limiting the communication to only Strategies that the Bridge implements? I think enacting this control will facilitate a much better controlled UI layer. (Still working on TDDable activities...)

The DataLoadStrategy has been updated to this new form and looks like

/* package */ static abstract class DataLoadStrategy{
        public static final DataLoadStrategy DataChanged = new DataLoadStrategy(){
            @Override
            public void run(final TopStoriesActivityBridge bridge) {
                bridge.topStoriesActivity.notifyTopStoriesChanged();
            }
        };

        public static final DataLoadStrategy DataError = new DataLoadStrategy() {
            @Override
            public void run(final TopStoriesActivityBridge bridge) {

                new AlertDialogBuilder<>()
                        .init(bridge.topStoriesActivity)
                        .setTitle(R.string.top_stories_strings_alert_dialog_failure_title)
                        .setMessage(R.string.top_stories_strings_alert_dialog_failure_message)
                        .show();
            }
        };

I find it silly to pass in the bridge when it's defined in the bridge; but it's the cleanest way to implement this I fumbled about with. I'm fine with it; love to see cleaner.

With the strategies modified; now we need to work on the Mediator's loadTopStories
This currently is:

 /* package */ void loadTopStoriesData() {
        new HackerNewsAccess().topStories(new Callback<Stories>() {
            @Override
            public void onResponse(final Call<Stories> call, final Response<Stories> response) {
                topStories = response.body();
                topStoriesActivityBridge.notifyTopStoriesChanged(dataLoadStrategyFactory(response));
            }

            @Override
            public void onFailure(final Call<Stories> call, final Throwable t) {
                topStoriesActivityBridge.notifyTopStoriesChanged(dataLoadStrategyFactory(null));
            }
        });
    }

We no longer need to call to call a method on the bridge. We can refactor loadTopStories to be

/* package */ void loadTopStoriesData() {
        new HackerNewsAccess().topStories(new Callback<Stories>() {
            @Override
            public void onResponse(final Call<Stories> call, final Response<Stories> response) {
                topStories = response.body();
                dataLoadStrategyFactory(response).run(topStoriesActivityBridge);
            }

            @Override
            public void onFailure(final Call<Stories> call, final Throwable t) {
                dataLoadStrategyFactory(null).run(topStoriesActivityBridge);
            }
        });
    }

While a small change. This restricts all behavior of the UI to the Bridge layer. Previously, passing in a DataLoadStrategy would allow us to extend and do unexpected things. We want to create a pit of success. We're limiting the functionality available so that it's harder for us; or someone new to the code; to do things in a way we don't want. This completely blocks it.

A small thing that will be hard to notice... We removed a method. Previously our notifyTopStoriesChanged would execute the provided DataLoadStrategy and since we're bundling that up and executing from the Mediator; the below method - POOF

 /* package */ void notifyTopStoriesChanged(final DataLoadStrategy dataLoadStrategy){
        dataLoadStrategy.run(topStoriesActivity);
    }

A method gone; a test gone. We're performing refactoring to the design and reducing code. It's a beautiful thing.

Awesome Improvements

We've simplified the design of the code.
We've removed a method.
We've furthered encapsulation (of the bridge).
We're communicating intent better.
We're preventing unexpected behavior.

This very small step to use the Mediator as I envision is already demonstrating that it allows a control mechanism to drive the UI in only intended ways.

Of course as engineers working on the project, we probably know better - But... If new behavior is desired in the UI based on new information in the Mediator... There's a fair chance someone w/o knowledge of the system would extend the DataLoadStrategy as an anonymous class in the Mediator and pass it in. I'd probably do that to get things in place then refactor - Now? Not. Possible.

I feel a lot better about systems that prevent things being done in undesired ways. This feels like it will strongly direct engineers in that direction - A pit of success.

Tests

While not called out; I do run the tests frequently... No, Steve... Not as frequently as I should. Can I blame the lack of a good "nCrunch" equivalent for Android Studio? I do blame lack of a pair. Lacking a pair is my current punching bag. :)

All 102 tests pass; and TopStoriesActivityBridge still has full code coverage. Which does include the construction and invoking show() on an AlertDialog.

Summary

This is a shorter post than I expected. The TDD process and how it sets the code up makes refactors and these types of changes more minor; even though it's a complete change in the implementation of how things behave. Once I got the error test in place; I did't have to change the tests regardless of all the changes around running the strategy.

The Mediator is starting to show the intent I have for it. It makes a selection of what operation to perform; and in this case, performs it. I originally had the execution in the Bridge as that's my original design.

Mediator selects, Bridge executes

Though I can see this is an extra hop that's not always required; Emergent design. Even if you have notions of the /how/, always be prepared looking if different is better.