Tuesday, February 19, 2008

Reaction to Actually Using the BGGA closure Prototype

I watched two talks from Javapolis 2007 a few nights ago on the Parley's site: Neal Gafter's Closures for Java and Joshua Bloch's The Closures Controversy. These were incredibly enlightening talks and well worth the time spent watching. One of the points I took away from Mr. Bloch's talk was that it is too early to settle into a JSR... there are too many unknowns about how the feature should work. Instead we should release the prototypes into the community and find out what the reactions are from the developers.

Thus charged, I found a real place to actually use the BGGA closure prototype and kept notes about what was easy and what wasn't. This is the result.

Recently, I've been working with the JConch Java concurrency library, specifically the CacheMap object (source). It's basically a thread-safe lazy map that caches each lookup. From the documentation, it is a data structure to use "when the cost of looking up a keyed value is so severe that it's worth spending some memory to cache commonly used values. The most obvious example is in database look-ups, but DNS look-ups and potentially expensive data validations are other good cases."

One simple example is a CacheMap that caches the result of string validations, taken from the project's examples.
CacheMap<String, Boolean> validationCache = new CacheMap<String, Boolean>(
new Transformer() {
public Object transform(final Object stringToValidate) {
if (stringToValidate == null) {
return false;
}
return !StringUtils.isBlank(stringToValidate.toString())
&& !StringUtils.isAlpha(stringToValidate.toString());
}
}
);
The CacheMap constructor takes an object of type Transformer, which will probably be supplied as an anonymous inner class, as this example shows. This can all be verified with the following simple assertions:
assert false == validationCache.get("");
assert false == validationCache.get("A");
assert true == validationCache.get("2");
assert true == validationCache.get(2);
Here is how the same object construction looks using the BGGA Closures proposal:
CacheMap<String, Boolean> validationCache = new CacheMap<String, Boolean>(
{ Object target =>
boolean result = false;
if (target == null) {
result = false;
} else {
result = !StringUtils.isBlank(target.toString())
&& !StringUtils.isAlpha(target.toString());
}
result
}
);
I'm not counting, but the closures version is actually 1 line of code longer. Let's break it down:
  • "Object target => " is certainly more concise than declaring a whole method and parameter list. I like it.
  • "boolean result" is needed because a local return can only be performed once at the end of a closure. This is where the additional lines come from
  • "result" with no semi-colon and no "return" keyword is needed to perform the local return. Using the "return" keyword would result in a non-local return.
So what does all this mean?
  • Converting my code to use the BGGA closures was quite simple
  • ... but it took more code than an anonymous inner class.
  • Remembering to use a local return and not use the "return" keyword was easy
  • ... but I just recently made that exact mistake a few days ago
  • ... and using closures is a novelty so of course I'll remember!
  • Declaring the parameter types was simple, no issues there
  • ... but having to locally return at the end of the closure is annoying
  • The closure conversion that allowed me to use a closure as a Transformer object was really cool. This is a library that is not written for closures, yet I could use them.
  • And why anybody would complain that "=>" was used instead of "->" is beyond me. It seems pretty inconsequential.
What would I change (if I could)?
  • Having "return" default to a non-local return is confusing. I'd rather "return" perform a local return and introduce a new "non-local-return" keyword for the special case
  • Only returning once is an annoyance that eliminates the possibility of guard clauses and forces an extra variable declaration. I wish this wasn't required.
For this use case, where I am a library user, I'm not sure closures gained me anything. It really was just as easy to declare an anonymous inner class.

It's absolutely wonderful that there is a prototype to play with. Thanks everyone who contributed to it, whoever you are! Source code for post available here.

2 comments:

Neal Gafter said...

Looks shorter to me:

CacheMap<String, Boolean> validationCache = new CacheMap<String, Boolean>(
  { Object target =>
    (target != null) &&
    !StringUtils.isBlank(target.toString()) &&
    !StringUtils.isAlpha(target.toString())
  }
);

Hamlet D'Arcy said...

hmmmm... I tried to leave the sample code structured the exact same way with or without the closure so that I could make an "apples to apples" comparison. But in doing so I ignored the style/idiom shown above that I could have used to shorten it. In hindsight, the constraint of not modifying the validation code seems silly.

Perhaps there will develop an in-closure coding style that will reduce the impact of returns and such; and the goal of moving code into a closure without change will be seen as a strawman.

Thanks!