Thursday, January 04, 2007

Dynamacism

The Pragmatic Programmer contains the outstanding piece of advice that you should learn a new language every year. It not only gives you more tools to apply to problems, but it encourages you to think in different idioms. Every language has its own set of patterns and styles (the original Kernighan and Ritchie book on C, known affectionately as the "K&R" book) was as much about how C should look as what it did. People still talk about the "K&R style"; they defined the idioms of C along with the language. Learning new idioms and styles makes you a better developer, thus the efficacy of the Prags advice.

Here's a recent example that I perpetrated. After a stint in .NET and Ruby on Rails, I'm back on a Java project for a while (and coding again in my beloved IntelliJ, even if it is on Windows). I ran across some code that caused my refactoring alarm to go off. Here's a snippet:
  
public void record(String toBeRecorded) {
MessageWriter messageWriter = null;
try {
messageWriter = new MessageWriter(_destination);
messageWriter.write(toBeRecorded);
} catch (RuntimeException rx) {
throw rx;
} finally {
if (messageWriter != null)
messageWriter.close();
}
}
public void record(List<String> listToBeRecorded) {
MessageWriter messageWriter = null;
try {
messageWriter = new MessageWriter(_destination);
for (String s : listToBeRecorded)
messageWriter.write(s);
} catch (RuntimeException rx) {
throw new RuntimeException("Cannot write list of messages");
} finally {
if (messageWriter != null)
messageWriter.close();
}
}

This is an overloaded method that does basically the same stuff before and after the important stuff (writing a single message or a list). Because I've been in languages with closures for a while, I refactored it thusly:

public void record(final String toBeRecorded) {
recordMessage(new Runnable() {
public void run() {
_messageWriter.write(toBeRecorded);
}
});
}
public void record(final List<String> listToBeRecorded) {
recordMessage(new Runnable() {
public void run() {
for (String s : listToBeRecorded)
_messageWriter.write(s);
}
});
}
private void recordMessage(Runnable recordingStrategy) {
try {
_messageWriter = new MessageWriter(_destination);
recordingStrategy.run();
} catch (RuntimeException rx) {
throw new RuntimeException("Cannot write list of messages");
} finally {
if (_messageWriter != null)
_messageWriter.close();
}
}

This is basically just the Command Design Pattern. But before I was using closures on a daily basis, I never would have thought to implement it this way. I would have created some additional classes in their own files, created a formal implementation of the Command pattern, and a lot of other excessive ritual. Of course, if this got much more complex, it might need that ritual but, in this case, I was able to consoliate all that open and close code in a single place (which made things DRYer as well).

6 comments:

bcarlso said...

I too have used annonymous interfaces in Java to accomplish the same task, which I wouldn't have considered until using a language that naturally supports them (In my case, Python). I'm not sure I would use that approach in this case. Generally when I have this situation I would simply delegate first call to the second by using:

this.record( Arrays.asList( new String[] { toBeRecorded } ) );

Anonymous said...

Nice demo of how to do Ruby-style blocks in Java. You may want to note that messageWriter has been changed from a local variable to a member variable (also implied by the underscore prefix). Also, is there any overhead to creating a new Runnable on every call to one of the record methods? Perhaps the two Runnables could be static member variables?

Anonymous said...

Oops, of course the Runnable must be created within the record method, otherwise it can't see the method parameter. Sorry. "Premature optimization is the root of all evil in programming" (Knuth, quoting Hoare)

Neal Ford said...

Yes, alas, one of the problems (as I'm sure you all know) of blogging code is that you sometimes leave out interesting details. Brandon is correct: in the case I provided, record( Arrays.asList( new String[] { toBeRecorded } ) ); would be a better solution. But, the messy real world intrudes, and for reasons to complex (and, frankly, stupid) to go into here, we ended up with the closure-style approach.

Using the closure approach does give you some additional benefits, just like the Strategy pattern: it's easy to define new inner classes that do all sorts of stuff and still leverage the init and cleanup code.

bcarlso said...

Neal Wrote:
> But, the messy real world intrudes, and for
> reasons to complex (and, frankly, stupid)

Of course! What else is there?

The biggest deterrent for me using "closures" more is the type system. I see the opportunity to use it all over, but I need to create a new interface just to use the technique in all but the most simple of cases. I've toyed with the "block" interface:

interface Block {
Object execute( Object... args );
}

Unfortunately, the mere thought of all of those casts makes my skin crawl. ;-)

Have you looked at the JSR for closures in Java? I haven't looked but was wondering how clean it was.

Thanks!
Brandon

Anonymous said...

You also might have more issues with thread safety. By pulling _messageWriter out as a member variable, as stephen mentioned, you're risking two threads clobbering each other. Might make sense to attach your messageWriter to the Runnable, possibly by implementing the Runnable interface rather than using anonymous classes.