A Not Very Groovy Design Problem
I mentioned that writing code in Groovy that looks like this:
myObject.”$localVariable”.attribute = 42
… is a bad idea for three reasons.
Here’s the three reasons I was looking for:
1. Violating encapsulation
You don’t want to manipulate the guts of other objects directly. If we made a habit of just reaching into the objects near the current chunk of code and doing whatever we want, we’d soon have a codebase which is incredibly difficult to change because whatever piece of code you’re looking at now could be potentially affected by another piece of code writing over local state.
Unit tests work by exercising the public interface of the object i.e. those services that are made available to the community of other collaborating objects in the system. If any old thing can reach in and mess around with the internal state then there wouldn’t be any point in having unit tests because you can’t rely on the assumptions on which those tests are based.
Every programmer learning his or her first object language used to be taught encapsulation, but I’m still amazed how many really experienced people still don’t seem to take this seriously. It’s not an aesthetic thing. If you don’t get this please give up.
Incidentally the Sharble and Cohen study from Boeing provides some evidence that designs that use lots of getters and setters to centralise design in a few classes also makes the code slower because you need to execute more instructions just to (needlessly) push all the data around.
2. It makes the object graph very fragile
This is about the Law of Demeter. Basically when you write the expression myObject.x.y.z.doWhatever() you’re assuming that there’s a whole bunch of things stuck together, at the end of which is an object that can doWhatever().
The code “myObject.x.y.z.doWhatever()” makes a lot of needless assumptions:
- myObject knows x
- x knows y
- y knows z
… and all it really needs is to be given z as a parameter.
If any of the relationships between anObject, x, y, and z changes we get an entirely preventable breakage.
Some people might find it difficult to restructure this code so that you get access to z without rummaging through an object graph (what some people still seem to call a “data structure”.) See what I’ve got to say below about responsibility-driven design to help with this.
3. It’s a gratuitous abuse of metaprogramming.
In other words, it’s slower and far more complicated that it should be. Incidentally, the business with .”$whatever” is hiding $whatever from compiler because it needs to be evaluated at runtime rather than compile time. It also makes things an order of magnitude more difficult for a refactoring IDE. I doubt that there’s an easy way for the IDE to refactor things automatically when you rename an attribute or to even notice that something’s wrong when you move the attribute to another class.
anObject.”$whatever” = 42
The metaprogramming here with .”$whatever” only became necessary because we’re trying to poke away at the guts of another object. That’s something we shouldn’t be attempting to do in the first place. We should get anObject to do something rather than just treat it as a passive store of data (what some people used to more honestly describe as a “record”.)
A funnier treatment of points 1 and 2 can be found in The Tragic Tale of POTS and his friend SPOT. Grails developers who’ve read about POTS will appreciate the irony of having a DogWalkingService.
How to fix this mess
The best way to avoid writing code like this is to think in terms of “responsibility-driven design” - that’s the art of putting the data right next to the code that needs it — in the same class.
The people that invented extreme programming knew about this. In fact RDD was invented by the people that invented extreme programming. Shame it’s not so widely used or publicised.
Shuffling responsibilities around between classes to resolve this instead of punching holes in encapsulation with metaprogramming means learning to write OO code that is:
- is faster (fewer method calls, more opportunity for compile-time optimisation and no compilation at runtime)
- is simpler,
- has more understandable class dependencies
- … so it breaks less often.
Doing CRC sessions is a very agile-friendly way to do RDD - and was done on the first ever XP project C3.
A lot of this stuff seems obvious (at least points 1 and 2). It’s surprising how many people don’t seem to get this.
Extreme programming is founded on the concept of lowering the cost of change. The points I’ve made here aren’t theoretical or style-related: This is vital. This is about the basic stuff that makes everything else we do possible.