It's OK to Almost Repeat Yourself

One of the key tenets of modern (and not so modern) software development was catchily coined in The Pragmatic Programmer as the DRY Principle, Don't Repeat Yourself. This principle states that any information should only be defined once in an application, and that definition should have total authority. It doesn't matter where that definition is, but it should only be in one place. For instance, if you want to define VAT as 20% in your accountancy application, then that should only be defined once, and every reference to VAT should be a reference to the original definition. This principle should be applied at every level, so that every piece of code that does something of significance should only be written once (no copy-and-paste - that's bad).

In this blog, I'm going to say something absolutely heretical in the world of development and probably with it sink my reputation (if any scrap is left of it) to non-existent in the eyes of anyone who has any kind of education in software development. That the DRY principle can drive inexperienced developers into creating unmaintainable code.


Now you are thinking: how is this crazy guy from the internet going to justify such an outrageous statement, but more importantly why is he allowed access to a computer keyboard?

Both good questions. In my day-job I work on a sprawling Java web application that, put simply, allows people to complete online surveys. My thinking about this started when I had to go back to a much hated (although heavily relied upon) piece of code in our system. Let's call it the SurveyWrapper.

The SurveyWrapper uses a kind of distorted Wrapper/adapter pattern that adapts a Domain Object (the Survey - persisted using Hibernate) into a more useful View-model Object (as used by numerous JSPs, but also by code that exports surveys to Excel spreadsheets and other exports). The Survey object itself doesn't have much state but it has many collections of various linked objects (questions, answers and what-have-you). These attached collections might be quite large so we don't always want to fetch them from the database, so they are either not mapped in Hibernate at all, or they are lazy-loaded collections.

The SurveyWrapper contains some methods that call straight through to the wrapped domain object but also holds wrapped versions of the various linked collections and itself decides what to fetch as it populates them in its constructor. So far, so terrible.

Now the problem is that this class is used as a model for so many different JSPs, and therefore the requirements of the model vary hugely depending on its use. So there are many constructors, each taking various boolean flags and constants in order to tell it how to populate itself. Along with the long constructor definitions, there is also a mess of if-statements in order to create the desired state for a million different use-cases.

What would be a better solution? Obviously to have different model classes for each view, each adapted perfectly to expose only the data that's required by the view it is designed for. When those views change (as they will), only the adapted model needs to change.

Now, in order to get there, at some point in the past a developer would have had to say 'Ok, I already have the SurveyWrapper, but it doesn't do what I want. So I'll create a new Wrapper that does what I want'.

The class they then create might be (at first at least) rather similar to the SurveyWrapper, and might look like the rightly hated code repetition. But it won't be the same, because even if the code is the same at the moment, the envisaged use of the class is different, therefore it should be a separate class.

Extending the existing SurveyWrapper should be ruled out as the subclass wouldn't be able to cleanly control the state of the superclass. The Decorator pattern might seem appropriate here, but to me it does not really fit since the views are likely to require different profiles of the data, rather than a super-set of a base model.

Instead of wrapping the Domain Object and directly calling through to getters on it, the model object should define its own state (I recommend using the constructor to build this state from the Domain Object, or directly from the database). This will seem to violate DRY, since you will define some of the same fields in the model as were present on the Domain Object. However, the pay-off is that this way you can guarantee complete separation from the Domain Object, and therefore from the database, and instead couple the model closely with the view that will make use of it, resulting in much cleaner JSPs and cleaner code in general.

This I think highlights one of the things I have learnt when creating my own application without a predefined framework, that you should always concentrate on creating decoupled code, instead of just separate layers, which can allow too much coupling within the same layer or hidden dependencies between layers. And don't let DRY scare you into creating multi-use monsters!