Thursday, April 26, 2007

Guard Clauses Considered Harmful

A colleague and I have had a long running debate on Guard Clauses, which is an example of "Alligators versus Crocodiles". A previous post tries to present the "for" case.

Now, it's hammer time.

IMO, the canonical form for a Java method is like so:


public MyObject doSomething(Object a, Object b) {
    MyObject result = null;

    // stuff goes here

    return result;
}


It's old-school: a single point of return. Any checks should concentrate on the positive and relegate error messages, exceptions, etc to the else statements at the bottom of the method.

For example:


if( a != null ) {
    if( b != null ) {
        doSomething();
    } else {
        // throw exception
    }
} else {
    // log error
}


Here are the advantages:
  • The logic of the method is easy to understand: set the default case first, then change it if necessary. It quickly becomes a pattern that makes reading and debugging code easier.
  • It is easy to enforce. There is no slippery slope as with guard clauses. Guard clauses easily degenerate into "open season" for returning from methods all over the place. This is much harder to maintain, and is more prone to bugs, than the (admitted) indentation issue. IDEs can help match braces etc with respect to indentation: may heaven help you if you need to understand a method with several return methods. Note that the example of guard clauses (here) is pristine. It's not always this way.
  • Stating the above in a different way, it's easy to scan and find the "heart" of the logic. The corner cases of exceptions and error logging is at the bottom of the method. Guard clauses puts that stuff right up front, which strikes me as being inverted.
  • Using a convention is extremely powerful: note that methods that use this format are amenable to being processed by simple scripts etc. e.g. If one wanted to insert "pre"/"post" logic logging (though admittedly this is surpassed by using AOP).
Counters to the "for" case:
  • In the purest form, guard clauses are brief statements at the top of the method and, yes, they can be thought of as a pseudo-contract. But when scanning through the method, there is something jarring about seeing a return statement. It invariably opens another thread in my head for that case, and as the returns stack up, so too do these threads. It is distracting.
  • An oft-cited argument for guard clauses is that methods in Java are shorter. Hmmm.... maybe. Example methods are shorter, and the pristine methods you and I write are shorter, but does the average Java Joe/Jane really write shorter methods than, say, C++? IMO, the above convention does not harm short methods but definitely assists the reader with the dreaded longer methods. The proper use of guard clauses is just too subtle for Java Joe/Jane.
And there you have it.... As always, I (and my colleague, who is no doubt reading closely) am interested in your feedback.

Do you see guard clauses as an alligator or a crocodile?

ps. I fully concede that this convention doesn't have a sexy term like guard clause and does not get press in popular books. Perhaps fans of this idea can give it a snazzy name (Return on Rails?)

Monday, April 23, 2007

The Case for Guard Clauses

As mentioned in a previous post, an XP pair partner of mine and I would get into debates about Guard Clauses. This is a classic Alligators versus Crocodiles issue.

The idea is, in essence, to return from a method quickly for illegal arguments before hitting the main logic of a method.

Guard clause:

if( a == null ) { return; }
if( b == null ) { return; }
doSomething(a, b);

versus not:

if( a != null && b != null ) {
doSomething(a, b);
} else {
// log error
}

I should probably ask him to "guest host" on this entry, but here's my best, honest attempt to summarize the arguments for guard clauses:

  • It's time to bid the 1980s a fond farewell. Methods do not have to adhere to the rule of "having a single point of return", especially in Java, because methods are much shorter now.

  • Checking for null etc ultimately leads to serious indentation which is hard to read.

  • When done correctly, the guard clauses read like a contract for the method. It's like Checked Exceptions Lite: all the intent without the "propogation" aftertaste.

  • It's a refactoring pattern with a cool name!

So there's the Alligator Assessment... next up: the Crocodile Case.

Sunday, April 22, 2007

Alligators versus Crocodiles



Marvin, it's back! Get the book... we'll settle if this is an alligator or a crocodile once and for all!


This is a favorite Far Side cartoon, as it is highly relevant to geeks and especially pair-programming.

I have started using the term "alligators versus crocodiles" for contentious debates on issues that don't merit the passion, and yet are not trivial "religious wars" (e.g. brace placement).

A former XP pair-partner and I have a dandy "alligators versus crocodiles" issue. We had a clear disagreement on it, and every so often our daily work would suddenly ignite into a (somewhat) heated debate. As one wag put it:

The passions are so high, because the stakes are so small.


Our particular issue involves Guard Clauses. Soon, I hope to post argument for and against this refactoring pattern.

Wednesday, April 18, 2007

Blog on blogs

This month's Wired magazine has several neat articles on the "transparency effect" of corporate blogs.

This one is a great article on Channel 9, an upstart blog that bubbled up from within Microsoft and is gaining serious "street cred" with developers and techies.

The Factoid:

Channel 9 was used by United Airlines for anxious passengers to listen to the pilot and air traffic control. The idea is that anxiety can be mitigated by knowledge and transparency. The name, "Channel 9", for Microsoft's blog reflects the original intent to show developers what is happening inside The Death Star in Redmond.

The Quote:

"Who told you you could do this? I want a meeting with your VP," read an email from a marketing executive in the Windows division. "Some of the information in the last video was false. Do you realize shareholders could sue us over this?" an attorney pinged. And then there were the dozens of awkward hallway run-ins: Someone in public relations or marketing would stop Pryor and ask, in effect, "Who do you think you are?"

By then, Pryor had been at Microsoft seven years. He spent the last two as director of platform evangelism - building good relations with outside code writers. For Channel 9, he had a powerful line of executive support running all the way up to Jim Allchin, then head of the company's giant Windows division. But in the face of some of the "nastygrams," as he calls them, Pryor wondered how long that support would last. "That first month I probably had 10 to 15 near-death experiences where I thought I was going to be fired," he says.

Tuesday, April 17, 2007

A Thought on Geek Presentations

Long-time readers will know that I have attended several No Fluff, Just Stuff conferences. I also attend the St Louis Java SIG (though I missed this month).

One thought has recurred to me during NFJS 07. What I remember most vividly from a given geek session is not a new language feature, Powerpoint slide, or snazzy demo: I remember ideas. I think that's an object lesson for all of us as speakers (and as writers when we comment our code).

Sunday, April 8, 2007

Syntactic Sillyness

Can you write a Java program where this is legal?

HaveYouSeen<? super Size> Me;

Inspired by the delightful book, Java Puzzlers

Monday, April 2, 2007

Preventing Project disIntegration

Loose Definitions

Let's define unit-testing as the testing of a small, targeted selection of classes as part of development. Unit-tests should be fast and run continually as part of development. Because of the need for speed, unit-tests typically use mock objects so that they stay within the app's process boundary.

When I first wrote unit-tests, I broke this rule all the time. I would write tests that crossed as many process boundaries as possible: touching a real DB and all the way back. After many arguments with people, I eventually realized that these were integration tests and not unit-tests. As a unit-tester, I was being evil by writing long-running tests that slowed things down and missed the point of unit-testing.

Rockin' with the Mockin'

Then, for a time, I went the other way: all unit-tests, all the time. The mantras were "Speed is king!" and "Who cares about the DB when you are concentrating on a composite-flyweight-visitor pattern?"

Well, I know someone one a project who cares about the DB: the project is working through some serious i18n issues. In my estimation, the problem lies with too few integration tests.

Unit by day, Integration by night

My new philosophy is the best of both worlds. The famous, celebrated unit-tests should be run during the day, as always. The nightly build should not only run these tests but also a serious suite of integration tests.

These tests should expressly cross system boundaries and deal with more with "use-cases" than mere "test-cases". Unlike unit-testing, where (semantic) redundancy should be avoided, I think redundancy should take a back seat with integration testing (i.e. don't worry if the same thing is being tested more than one way -- this doesn't mean code redundancy).

The focus of integration testing should be in asking the big questions.

Here are some examples:

Question: Is my app internationalized?

Tactic: Using canned input in French/Russian/Japanese, a test calls the app's service layer to store that input in a given object/DB, and then reads it back through the service layer to ensure that i18n is stored correctly.

Question: Does my app handle a given use-case? e.g. In a content-management system, a user adds a resource to a document, and is warned that the resource is being shared and cannot be modified. The user continues to include the resource and tries to modify it anyway.

Tactic: Use a testing DB to set up the raw materials for these kinds of use-cases, possibly restoring it after every test (slow, yes, but these will be running at 4 am!) Then integrate individual test-cases through composition to re-create the story above. The finished test will probably read like a narrative that describes the above scenario.

Bottomline

We don't want surprises late in the dev cycle, or else our fine projects will disintegrate into chaos. The rock-star Unit Test should be given its due, but don't forget its older brother Integration Test.