Closure property conventions for events = anti-pattern
Having had an interesting time recently playing with flows and GORM events in a couple of Grails applications, as well as looking at some plugins like the Audit Logging plugin, I have some views to share on the use of Closure property conventions.
Background:
In Grails usually the first time you encounter the use of closures as property values is when you are defining actions in a controller, or constraints on a domain class.
This is a pretty good mechanism. It has some problems – regarding inheritance for example – but these are mainly for the “back end” framework code. It affords the framework more flexibility in the way it provide dynamic methods and properties, rather than just using the metaClass of the controller to do this.
The problems:
The difficulties with this approach start to show up with things like GORM events. If you try using domain classes with events in flows you start to discover all kinds of issues that are side effects of using closure properties instead of methods. In summary;
- Inheritance is broken. In a closure property you can’t call “super” – so unless the framework provides a mechanism for it, the closure code cannot call the closure from the ancestor class. Frameworks have to also deal with this problem in terms of gathering up all the closures in the hierarchy and merging the results of them eg constraints in GORM – a special case where the code is DSL. This is a problem for more general code eg event handlers.
- Serialization problems. The programmer will find out at some point that closure properties should not be serialized. This means marking the properties “transient”. Except that you then find that if you do this, after loading from serialized form, the event property is then null. So actually you have to flag it as “static” and not “transient” to get around this – which restricts what the closure can do. For GORM you also have to explicitly add these properties to the “transients” convention property to stop Hibernate trying to persist them. If you have plugins that expect other closure properties then you also have to mark these as static and in the transients list also. No matter that Groovy 1.6.x will support Closure serialization. You should never be serializing event code in normal applications.
- Without reading docs you don’t know what “delegate” is in such a closure, and “delegate” does not read as well as “target” or “this” etc
So what I’m getting at is, I now strongly believe that using closure properties for “events” on objects is generally bad news. The best solution is good old fashioned virtual methods. These support “super”, have no inheritance issues, and no serialization issues.
For example the Audit Logging plugin uses closure properties for events on domain classes, and when evaluating this plugin I immediately thought “Hmmm will it be handling inherited handlers properly or not?” and of course there’s no info in the docs, and there would be code overhead in the plugin to support this – eg instantiating once (nonsensically) every domain class to evaluate its convention. This is something that Grails did (or still does?) do for services which causes all kinds of weirdness if you expect your service to only instantiate once in an application.
Furthermore in GORM, are inherited beforeXXX events on GORM classes called if you define a new one in a descendent? These are the kinds of questions that crop up. You readily assume they will, but you don’t know until you try.
I’m hoping that in Grails 1.1 or soon after the beforeXXX GORM events will transition to methods to avoid all the problems relating to (1) and (2) in the above list.
I’m also hoping plugin developers will stop using closures for event handlers. I’ve done it myself – in the Authentication plugin – but here it is a different case. The events are stored in a singleton map and are meant to be mutable and set by the application. There are no inheritance issues, nor serialization gotchas.
Death to per-instance closure properties for event handling!
