Since the first version of this post I hit on a much tidier solution, and the post has been updated to reflect this.
I was writing a renderer yesterday. I haven't done this in a long time, but ended up at a problem I remember not finding a nice solution to before (probably c. 2000!). I think that the solution I came up with yesterday was nicer than 14 years ago, which is good. However, Java 1.7 didn't let me do quite what I wanted...
I've got some features to draw. Using inheritance they derive from a base class. The features are created by parsing some input and may be numerous and long lived. The details are unimportant here, so we'll reduce them to:
abstract class Feature { ... }
class FeatureA extends Feature { ... }
class FeatureB extends Feature { ... }
I have a separate rendering class. This walks over the features and draws them, as you'd expect. I want to be able to decouple this from the building of the feature set. When I say decouple I mean separate machines, possibly repeated renderings, possibly using different rendering engines on the same data over time. One intuitive (to me) way to write this is:
class RenderEngine {
List features;
void render() {
setUpGraphics();
for (Feature f : features) {
renderFeature(f);
}
outputResult();
}
private void renderFeature(Feature f) {
// Feature is abstract, this just catches unimplemented renderings
}
private void renderFeature(FeatureA f) { ... }
private void renderFeature(FeatureB f) { ... }
Of course this doesn't work! Only
renderFeature(Feature f) gets used. Java doesn't give me choose a method implementation by the dynamic type of the arguments within an object.
So, what to do?
I really don't want to put the rendering code into Feature - it is busy enough describing the feature and helping the parser abstract from the raw data.
The Decorator pattern isn't really applicable as Feature and rendering have different purposes and different base classes.
I could use a sequence of test, cast, calls:
if (f instanceof FeatureA) { renderFeatureA((FeatureA)f); }
It worked. But, to me, it also smells. Long lists of tests for classes feels like I've used inheritance then broken it, and it would be easy to muck up adding features later. Also, all these tests would be going on in a loop.
The next solution I ended up with is influenced by the fact that all features of a given class will be rendered the same. No exceptions. The
Command pattern is the starting point. I can refactor my rendering methods into rendering classes. The methods only use a very little state from the RenderEngine class so the objects will be easy to set up. There will be a
lot of feature objects of few types so I don't want to tell each object about the renderer individually - especially as I can't be sure of the renderer at creation time. The trick was to use a static variable to hold the reference to the rendering object from the feature class. However, note that the following is also "broken" as a solution:
abstract class Feature {
static RenderEngine render;
}
The sub-types use the one static variable in Feature so both return whichever one is assigned last. This is also the downside of the approach - if the rendering of a feature depends somehow on the oveerall rendering of that task and there may be multiple concurrent tasks then the statics are broken.
Java 8 would give abstract static, but I'm not using that here (yet).
At the point the rendering object is assigned the class of the feature is definitely known. I can just use a method in the feature sub-types to assign to a field of those classes. Using inheritance to avoid the duplication would be better, but the code is trivial. The getter is then defined as abstract in Feature and written for each Feature sub-type. So I end up with code roughly as follows:
abstract class Feature {
abstract FeatureRender getFeatureRender();
}
class FeatureA extends Feature {
static FeatureRender renderA;
static void setRenderObject(FeatureRender ro) {
renderA = ro;
}
@Override
public FeatureRender getRenderObject() {
return renderA;
}
...
}
class FeatureB extends Feature { ... }
abstract class FeatureRender {
abstract void render() ;
...
}
class RenderA extends FeatureRender { ... }
class RenderEngine {
List features;
void render() {
setUpGraphics();
RenderA ra = new RenderA( ... );
FeatureA.setRenderObject(ra);
...
for (Feature f : features) {
f.getRenderObject().render(f);
}
outputResult();
}
My Feature classes are now aware of rendering, but don't hold any of the code to make it happen. I have a sequence of calls to set the rendering before the render loop, and at each iteration I get the rendering object. I think that is more elegant than the sequence of test, cast, call that I had in the rendering loop before. Adding a new feature still involves adding a line of code to set the rendering object, although arguably cleaner and more efficient code than the instanceof test. One bonus of splitting the rendering methods out into their own classes is that the little collection of constants for each goes with it. None is a big step, but together they make RenderEngine much tidier than it was before.
The statics are a problem however, as this is now part of a web service, rather than a stand-alone application.
So, I might be rendering with two different rendering classes at the same time - for different outputs.
This required a change of direction. One which, having arrived at it, I quite like.
In essence, the RenderEngine takes over responsibility for mapping feature objects to rendering objects - but without the clumsy instanceof tests.
The solution looks roughly as follows:
class RenderEngine {
protected Map renderingMap;
...
public void setupRendering() {
RenderFeature rf = new RenderFeatureA(sizing);
renderingMap.put(FeatureA.class, rf);
// and similar for other features, also set any parameters due to overall document
}
private void renderFeatures(Graphics2D g2d) {
for (Feature f : features) {
renderingMap.get(f.getClass()).render(g2d, f);
}
}
Also, this approach has greatly simplified the various feature classes: they no longer need to refer to rendering at all.
This means that the rendering is in a more coherent group of classes and the coupling between the feature data and feature drawing is much reduced; in particular the data doesn't need to be aware of the drawing any more.
This decoupling would also have been present in my initial, impossible, approach - it was only adopting the command pattern that created it.
And the obligatory testing note: I was writing tests as I went. But
unit testing graphics is hard so it didn't pick up the failure to draw the right thing. Now I have it working I could test by example, but first time round I couldn't. Even so, I did have a test of running the rendering so it made a tight cycle of refactoring through the various solutions with the aid of a logger and debugger. I could quickly see when it worked and when it didn't, and when my changes broke something else.
