From d69cb03c79e43a6ad399a033d7635aec07b6a75f Mon Sep 17 00:00:00 2001 From: Patrick Forhan Date: Sun, 1 Apr 2012 23:57:59 -0700 Subject: [PATCH] MVC classes --- .gitignore | 2 + .../newsreader/mvc/ArticleController.java | 7 ++ src/sans/newsreader/mvc/ArticleDisplay.java | 9 ++ .../mvc/DefaultArticleController.java | 26 ++++++ .../mvc/DefaultNewsreaderController.java | 82 +++++++++++++++++ .../newsreader/mvc/NewsreaderController.java | 14 +++ .../newsreader/mvc/NewsreaderDisplay.java | 15 ++++ src/sans/newsreader/mvc/philosophy.txt | 90 +++++++++++++++++++ 8 files changed, 245 insertions(+) create mode 100644 .gitignore create mode 100644 src/sans/newsreader/mvc/ArticleController.java create mode 100644 src/sans/newsreader/mvc/ArticleDisplay.java create mode 100644 src/sans/newsreader/mvc/DefaultArticleController.java create mode 100644 src/sans/newsreader/mvc/DefaultNewsreaderController.java create mode 100644 src/sans/newsreader/mvc/NewsreaderController.java create mode 100644 src/sans/newsreader/mvc/NewsreaderDisplay.java create mode 100644 src/sans/newsreader/mvc/philosophy.txt diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..6ec5795 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +gen +out diff --git a/src/sans/newsreader/mvc/ArticleController.java b/src/sans/newsreader/mvc/ArticleController.java new file mode 100644 index 0000000..9f27aa7 --- /dev/null +++ b/src/sans/newsreader/mvc/ArticleController.java @@ -0,0 +1,7 @@ +package sans.newsreader.mvc; + +/** Controller interface for the ArticleActivity. */ +public interface ArticleController { + void setDisplay(ArticleDisplay display); + void onCreate(boolean hasTwoPanes, int categoryIndex, int articleIndex); +} diff --git a/src/sans/newsreader/mvc/ArticleDisplay.java b/src/sans/newsreader/mvc/ArticleDisplay.java new file mode 100644 index 0000000..b486a1b --- /dev/null +++ b/src/sans/newsreader/mvc/ArticleDisplay.java @@ -0,0 +1,9 @@ +package sans.newsreader.mvc; + +import sans.newsreader.core.NewsArticle; + +/** The framework implementation of an article display. */ +public interface ArticleDisplay { + void finish(); + void displayArticle(NewsArticle article); +} diff --git a/src/sans/newsreader/mvc/DefaultArticleController.java b/src/sans/newsreader/mvc/DefaultArticleController.java new file mode 100644 index 0000000..7ffd432 --- /dev/null +++ b/src/sans/newsreader/mvc/DefaultArticleController.java @@ -0,0 +1,26 @@ +package sans.newsreader.mvc; + +import sans.newsreader.core.NewsArticle; +import sans.newsreader.core.NewsSource; + +/** SANDROID note no android classes in use. */ +public class DefaultArticleController implements ArticleController { + private ArticleDisplay display; + + @Override public void setDisplay(ArticleDisplay display) { + this.display = display; + } + + @Override public void onCreate(boolean hasTwoPanes, int categoryIndex, int articleIndex) { + // If we are in two-pane layout mode, this activity is no longer necessary + if (hasTwoPanes) { + display.finish(); + return; + } + + // Display the correct news article. + NewsArticle article = NewsSource.getInstance().getCategory(categoryIndex) + .getArticle(articleIndex); + display.displayArticle(article); + } +} diff --git a/src/sans/newsreader/mvc/DefaultNewsreaderController.java b/src/sans/newsreader/mvc/DefaultNewsreaderController.java new file mode 100644 index 0000000..22b94e0 --- /dev/null +++ b/src/sans/newsreader/mvc/DefaultNewsreaderController.java @@ -0,0 +1,82 @@ +package sans.newsreader.mvc; + +import sans.newsreader.core.NewsCategory; +import sans.newsreader.core.NewsSource; + +public class DefaultNewsreaderController implements NewsreaderController { + private static final int NO_ARTICLE = -1; + + // List of category titles + final String CATEGORIES[] = { "Top Stories", "Politics", "Economy", "Technology" }; + + private NewsreaderDisplay display; + // Whether or not we are in dual-pane mode + private boolean hasTwoPanes; + // The news category and article index currently being displayed + private int categoryIndex; + private int articleIndex; + + @Override public void setDisplay(NewsreaderDisplay display) { + this.display = display; + } + + @Override public void onCreate(boolean hasTwoPanes, int categoryIndex) { + this.hasTwoPanes = hasTwoPanes; + display.setUpActionBar(CATEGORIES, hasTwoPanes, categoryIndex); + } + + @Override public void onRestore(int categoryIndex, int articleIndex) { + setCategory(categoryIndex, articleIndex); + } + + private void setCategory(int categoryIndex, int articleIndex) { + this.categoryIndex = categoryIndex; + this.articleIndex = articleIndex; + NewsCategory category = getCurrentCategory(); + display.setCategory(CATEGORIES[categoryIndex], category); + // If we are displaying the article on the right, we have to update that too + if (hasTwoPanes) { + if (articleIndex == NO_ARTICLE) { + // Default to first article. + display.setArticle(category.getArticle(0)); + } else { + display.setArticle(category.getArticle(articleIndex)); + } + } + } + + private NewsCategory getCurrentCategory() { + return NewsSource.getInstance().getCategory(categoryIndex); + } + + @Override public void onStart() { + // This might have been 0,0 originally. + setCategory(categoryIndex, articleIndex); + } + + @Override public void onHeadlineSelected(int articleIndex) { + this.articleIndex = articleIndex; + if (hasTwoPanes) { + // display it on the article fragment + display.setArticle(getCurrentCategory().getArticle(articleIndex)); + } else { + display.showArticleActivity(categoryIndex, articleIndex); + } + } + + @Override public void onCategorySelected(int catIndex) { + setCategory(catIndex, -1); + } + + @Override public int getArticleIndex() { + return articleIndex; + } + + @Override public int getCategoryIndex() { + return categoryIndex; + } + + @Override public void categoryButtonClicked() { + display.showCategoryDialog(CATEGORIES); + } +} diff --git a/src/sans/newsreader/mvc/NewsreaderController.java b/src/sans/newsreader/mvc/NewsreaderController.java new file mode 100644 index 0000000..c0bb421 --- /dev/null +++ b/src/sans/newsreader/mvc/NewsreaderController.java @@ -0,0 +1,14 @@ +package sans.newsreader.mvc; + +/** Controller interface for the Newsreader. */ +public interface NewsreaderController { + void setDisplay(NewsreaderDisplay display); + void onCreate(boolean hasTwoPanes, int categoryIndex); + void onRestore(int categoryIndex, int articleIndex); + void onStart(); + void onHeadlineSelected(int articleIndex); + void onCategorySelected(int catIndex); + int getCategoryIndex(); + int getArticleIndex(); + void categoryButtonClicked(); +} diff --git a/src/sans/newsreader/mvc/NewsreaderDisplay.java b/src/sans/newsreader/mvc/NewsreaderDisplay.java new file mode 100644 index 0000000..03991a9 --- /dev/null +++ b/src/sans/newsreader/mvc/NewsreaderDisplay.java @@ -0,0 +1,15 @@ +package sans.newsreader.mvc; + +import sans.newsreader.core.NewsArticle; +import sans.newsreader.core.NewsCategory; + + +/** The framework implementation of an NewsReader display. */ +public interface NewsreaderDisplay { + + void setCategory(String title, NewsCategory category); + void setUpActionBar(String[] categories, boolean hasTwoPanes, int categoryIndex); + void setArticle(NewsArticle article); + void showArticleActivity(int categoryIndex, int articleIndex); + void showCategoryDialog(String[] categories); +} diff --git a/src/sans/newsreader/mvc/philosophy.txt b/src/sans/newsreader/mvc/philosophy.txt new file mode 100644 index 0000000..35a9fad --- /dev/null +++ b/src/sans/newsreader/mvc/philosophy.txt @@ -0,0 +1,90 @@ +Summary + +This is a proposition for how to construct Android code using a strong MVC sensibility. Since +View already has meaning in Android, and because Android reaches farther than only UI views, +think of it as a Model-Framework-Controller structure. + +Purpose + +"The only way to win is not to play" + +For some reason Android is a very hostile development environment. Some of this stems from +poor API design, others from the surrounding SDK and tools. Two obvious examples are the +preference for abstract and concrete classes in place of interfaces, and the outright hostility +to testing in general. + +To that end, I have constructed a lightweight philosophy for how to create android applications +whose logic is divorced from the Android code base. This allows for a strict separation of +view and controller and easy testability. + +New Code - Controller and Display + +Typically, for each activity, new code will consist of two interfaces and a class. Supposing +an activity was named PaintActivity, there would be: + - PaintController - interface defining all logic to be performed by an Activity. + - PaintDisplay - interface defining operations that interact with the user and/or Android. + Would normally be called PaintView, but this term is taken. + - DefaultPaintController - implementation of PaintController that uses methods on PaintDisplay + to perform its logic. + +And of course, PaintActivity would implement PaintDisplay. + +Mechanism + +Quite simply, all non-Android logic and state resides in the Controller implementation. The +Activity is reduced to view interactions and Android system interactions only. + +To convert an existing activity, start by moving its fields to the Controller. Then generally, +for each logic block, create a corresponding method on Controller. As needed, create methods +on the Display for the controller to call. + +The Activity will flatten out to simple methods implementing the Display. During onCreate, +it will set itself on the Controller, and pass any information necessary from intents or +bundles for the controller to initialize itself and the Activity. It will attach all +appropriate listeners as well. + +A key tenant is to keep all Android-related classes and method calls within the activity. +Application-specific domain classes may be passed back and forth between Controller and Display. +Application-specific logic, however, should remain confined to the Controller. + +Testing + +The Activity should be simplified to the point that it seems trivial. For example, it is +told to display a string in a field, and it does so. There are no branching paths based on +custom application state. It feels foolish to test. (Though full integration tests are worth +their own look.) + +Rather than writing a testSetTitle_setsTitle() method on the Activity (since that's all it has +now), the meat of the testing is upon the controller implementation. Simply provide it a stub +or mock Display implementation and you can write interesting tests like +testWhenUserHasNoName_promptsUserForName, all while avoiding android testing restrictions (no +stub classes) or heavy-handed testing frameworks (sorry, Robolectric) + +Criticism + +A simple issue with this approach is naming. As mentioned before, we can't use the familiar +"View" of MVC, and besides, with services and i18n and so much more offered by Android, it +is not really appropriate. I chose to use 'Display' but that also seems scope-limited. +"Framework" is a bit more accurate but at the same time something like UserFramework or +PaintFramework sounds a bit odd. Is there a better name? + +This proposal does add an extra layer to the software stack, though I maintain it is a +lightweight and practical one. Android seems to be trying to use fragments and sundries +to accomplish much the same goal -- the removal logic locked into Activities. This goes a +step farther. + +Finally, there is no single framework or appropriate design. Some controllers may have one +method, some ten. And I'm sure the code will differ based on the author's personality. + +Implementation + +I've performed a simple implementation [link] of this philosophy based on the Newsreader [link] demo from +the Android developer site. This is a reasonably complex demo that performs a number of +layout and fragment tricks. Still, it was a good starting point. It has two activities, +one simple, one complex. The fragments were thankfully just thin wrappers around their +views and didn't really have to be adjusted. + +I split the app into two packages, .core, containing domain objects and logic, and .ui, which +held android classes extending Fragment, Activity, etc [link to this commit]. I then constructed a third package, +.mvc, into which I placed the new Controller and Display interfaces and implementations. [link to that commit] +