-
Get a monthly update on best practices for delivering successful software.
Last time we came this way we looked at the misuse of a getter in the java.io.File. Not that it was wrong to have a getter in File, just that it led to misuse of that information in a UI control to determine what file encoding needed to be used to store the model.
This time we are looking at the Tally-ho CMS project. This is perhaps a more pernicious example of the shortcomings of getters and setters. We're not having to remember rules about changing state here (that's for an upcoming post), but we are exposing the implementation details and thus repeating code and making our application harder to change.
There are many places where ArticleBean is unpacked in the application. Here are just two examples.
First, from net.spatula.tally_ho.ui.article.ArticleHome we have
protected void populateItem(ListItem item) {
ArticleBean article = (ArticleBean)item.getModelObject();
Fragment headlineFragment = new Fragment(fragmentName, "headlineFrag");
BookmarkablePageLink articleLink = new BookmarkablePageLink("articleLink", ArticleRead.class, new PageParameters("0="+String.valueOf(article.getId())));
articleLink.add(new Label("title", article.getHeadline()));
headlineFragment.add(articleLink);
headlineFragment.add(new Label("author", article.getCreator().getUsername()));
headlineFragment.add(new Label("date", article.getCreateDate()));
headlineFragment.add(new Label("summary", article.getSummary()));
item.add(headlineFragment);
}
}
Note that we yank the state out of the article bean and its fields. And from net.spatula.tally_ho.ui.admin.ApproveArticles we have
@Override
protected void populateItem(ListItem item) {
if (!(item.getModelObject() instanceof ArticleBean)) {
return;
}
final ArticleBean article = (ArticleBean) item.getModelObject();
item.add(new Label("alias", article.getCreator().getUsername()));
item.add(new Label("title", article.getHeadline()));
final ModalWindow articleWindow = new ModalWindow("articleWindow");
articleWindow.setTitle("Article Display Window");
articleWindow.setCookieName("articleWindow");
articleWindow.setCookieName(null);
articleWindow.setUseInitialHeight(false);
articleWindow.setResizable(true);
articleWindow.setCssClassName(ModalWindow.CSS_CLASS_GRAY);
articleWindow.setContent(new ArticleDisplayPanel(articleWindow.getContentId(), item.getModel(), null));
item.add(articleWindow);
Again, we unpack the ArticleBean and jam its contents into something else. Multiply this by a dozen and you see the problem. Any change to the implementation of ArticleBean and we have to grovel around several code files and make changes to this code. At the very least it violate the DRY principle, at the very worst it puts us into Structured Design territory and the terrifying solutions to the awful problems it considers.
What are our options? This is a system boundary, i.e. we are going from model to UI, so we will have to expose the implementation at some point. Well, we could put this logic in ArticleBean and have it take a parameter to be modified. But that would put UI logic into the model. Not so good.
A better solution here would be a helper object that consolidates all of the logic in one place. Imagine a change to the implementation of ArticleBean (maybe we can stop calling it a bean, eh?), where we go to a variable state mode, i.e. a map of key-value pairs. Now we can change our helper object without having to change our two dozen or so collaborations with the ArticleBean.
OK, tag, your it. Disagree? I want to hear your arguments. Agree and seen similar things? I want to hear about that too.
Related posts:
Topics: Design Patterns, OOAD, OOP, Structured Design