OO Programming, instanceof, and Separation of Concerns

I know it’s been a while since I (or anyone else) posted here, but that’s not due to a lack of activity internally. We’re working hard on getting the latest versions of our products ready, and we’re still pushing out relatively regular point releases of Gosu. Hopefully in 2011 I’ll find the time to write a little more regularly.

There’s a fairly common aversion in the Object Oriented Programming community to code that uses the Java “instanceof” operator (or its equivalent in another language), and the aversion is reasonably well-founded: the instanceof operator often indicates a failure of encapsulation. For example, if you ran across code like this:

public void draw(Shape s) {
  if (s instanceof Circle) {
    drawCircle((Circle) s);  
  } else if (s instanceof Rectangle) {
    drawRectangle((Rectangle) s);
  } else {
    throw new IllegalArgumentException();
  }
}

it might be reasonable to say that you should instead move the draw() method onto the Shape class and have it implemented differently for different shapes. That has a number of benefits. First of all, it localizes the logic along with the class that’s going to be drawn, and presumably the author of the Shape in question, having implemented the class, knows how they intended for it to be drawn. Secondly, without that kind of encapsulation the author of the draw(Shape) method will need to update the method every time a new Shape is added, which means that two different parts of the system are now tightly coupled, and they’re coupled in a way that’s not easily discoverable. Third, it makes it clear to a new Shape author what’s required of them: if there’s an abstract method on the superclass (or if Shape is an interface), it’ll be clear that a draw() method needs to be implemented.

That’s a fundamental enough principle of good OO design that some people tend to take it a little far and start suggesting that all such if/else branches using instanceof are a bad idea, or even that all if statements themselves are somehow a failure of encapsulation. Unfortunately, that tendency can sometimes result in a cure that’s even worse than the disease: it can lead to a massive violation of the separation of concerns.

Separation of concerns refers to keeping distinct pieces of functionality within a given system as separated from one another as possible. (See http://en.wikipedia.org/wiki/Separation_of_concerns). In order to control the complexity of a larger system, it’s essential that the system be divided into pieces that interact and overlap as little as possible so that they can be worked on and understood independently. Taking OO programming too far, then, can violate separation of concerns by forcing code that properly belongs to one concern and jamming it into an area that’s part of another concern.

For example, suppose that instead of dealing with drawing shapes, the Shape objects that we’re getting were originally intended to represent elements being drawn in a client-side application, and now we want to add an alternative rendering engine that outputs Javascript. The OO-zealot way to do that might be to add in a method to each shape like:

drawUsingJS()

In this case, however, the Javascript-drawing layer is a separate concern from the Shape layer that is focused around constructing and manipulation shapes. Attempting to combine the two by pushing Javascript-specific logic up into the Shape classes would likely lead to an engineering disaster: the parts of the system would become tightly coupled, and if the Javascript team decided that they wanted to move to a different base Javascript library, everyone on the Shape team would have to rewrite their code and a huge amount of cross-team communication and coordination would be required. Instead, the Javascript team should logically be considered to be downstream of the Shape team and a client of their APIs (i.e. what methods are publicly exposed on the Shape objects), leaving them free to use those APIs in whatever way they need to in order to implement their functionality.

Of course, that could well leave them writing code that looks like:

public void drawUsingJS(Shape s) {
  if (s instanceof Circle) {
    drawCircleUsingJS((Circle) s);  
  } else if (s instanceof Rectangle) {
    drawRectangleUsingJS((Rectangle) s);
  } else {
    throw new IllegalArgumentException();
  }
}

which is exactly the sort of code that OO zealots (or the anti-if-statement crowd) will tell you is horrible. That’s not the only implementation option, of course. You could choose to create JS wrapper objects around each Shape class that themselves have a drawUsingJS() method on them . . . but then the construction of those wrapper classes might end up as a bunch of if/else/instanceof calls (unless you want to do it reflectively, which is a reasonable alternative but can lead to its own sorts of problems). You could also use double dispatch here, with a drawUsingJS() method on Shape that takes a JSDrawingContext object which has methods on it like drawCircle, drawRectangle, etc. . . . but then you end up in a situation where the two components are coupled to each other, and the Shape library has to be aware of the JS-drawing library, rather than just having a one-way dependency.

Plenty of other patterns and options exist for handling that kind of implementation problem, but my point is not to argue for any of them. Rather, my point is that there’s no easy answer and you’re going to have to make a tradeoff between, on the one hand, using traditional OO encapsulation techniques that tightly couple the libraries together and violate the separation of concerns, or you lose all the benefits of OO encapsulation while keeping concerns separate and ensuring that the dependencies in the graph flow only in one direction. In some circumstances, coupling things will be the right choice, and at other times keeping them loosely coupled will be the right one.

This is just yet another instance of the general rule that no particular programming approach or rule is itself infallible: you often have to trade off one sort of a problem for another and decide which sort of non-ideal state is less harmful. The task of managing a large software system is often reducible to the task of managing (and containing) a huge amount of complexity, and managing complexity is itself an exercise in making tradeoffs given imperfect information. And the fallacy with being too much of an OO zealot is not so much in making bad tradeoffs as it is in failing to acknowledge that there’s a tradeoff to be made, and that sometimes the balance is going to tip away from what might otherwise be seen as a “best practice” in order to avoid doing something even more damaging.