Adding functionality: subclass vs decorator

Question

I'm currently working on a legacy system using Oracle's ADF Faces JSF implementation for the presentation layer. The system relies on a rules engine to make certain form elements required, disabled, or even highlighted depending on the user's interaction with them or values entered.

At it's current stage the application is "working", sort of. The current implementation handling the rules engine and the front end updates isn't very elegant and consists of a large set of if statements similar to:

if(screenObj instanceof CoreInputText) {
    ((CoreInputText) screenObj).setDisabled(true);
}

To complicate the mix we also have our own objects mixed in so the set as a whole does not share a common ancestor thus eliminating our option of doing something like:

((CommonAncestor) screenObj).setDisabled(true);

The question is whether or not it would be worth reworking this portion of the code to be cleaner and clearer. Because the majority of the screen elements are ADF Faces elements we're not able/allowed to change their ancestry to add additional methods.

The goal of the code change would be two fold: clean up older code and improve the code base so that the addition of new elements or controls won't result in a large code change (specifically to the if statements which exist in numerous places).

So if we go forward with this change which would be the better choice of achieving what we're looking for? Subclassing all of the elements (requiring a new class each time we utilize another pre-existing control) or implementing the decorator pattern? My only concern with the decorator is that it would still require a code change for each additional element. Both options would appear to reduce the code changes since multiple methods containing these if blocks wouldn't need updating.

Any input on methods for handling such situations beyond subclassing and decorators is welcome!

Answer 1

Although they don't have a common ancestor, you don't specify if they have a common method. If they do, then perhaps just put in an interface at runtime using a Proxy that just invokes the method so you have one common type. This is a kind of decorator, certainly, and since it uses reflection it may be too slow (although likely not) but it has the advantage of any new objects just having to implement the right method signature (or perhaps have new objects just implement the interface, and reserve the Proxy for old objects and conditionally create the proxy in a factory method that checks if the object needs it).

Also, if this is a true setter, you could use BeanUtils from apache and just set it as a property called disabled.

EDIT: Given that they don't have the same method, some code is going to have to be added every time you add a new object, unless you can at least control that going forward new objects will have a defined interface. That being said, if you are comfortable going down the reflection route, you could create a map of classes to methods (provided the methods all take a boolean - and you could even force your way around that if you have to) and lookup the object in the map. That way when you introduce a new object you only need to add an entry in the map.

However, my preference would be to avoid that, as that seems to take reflection too far. It makes the method names very hard to refactor, and impossible for an IDE to figure out that they are used in that context. I think the decorator is the right choice at first blush because, in the abscense of any other consideration, composition should be prefered to inheritance and it will make a clearer code path as to what is happening, improving maintenance in the long run.