Closures - Last-line-No-SemiColon nastiness

The BGGA closures proposal currently plans to use a 'last-line-no-semi-colon' to return the result of the closure back to the closure-defining method. I personally find this rather nasty. What about an alternative?

Last-line-no-semicolon

Here is some code that traverses a complex object structure to find all the bookings that contain a specific purchase type. (Yes, I know its not OO, a lot of real world code isn't - OO isn't the point of this blog post).

List[Booking] matches = new ArrayList[Booking];
for (Booking booking : bookingList) {
  for (Purchase purchase : booking.getPurchaseList()) {
    if (purchase.matchesType(typeToFind)) {
      matches.add(booking);
      break;
    }
  }
}

And here is the code refactored to use a BGGA closure, 'findAll', using the current rules:

List[Booking] matches = findAll(Booking booking : bookingList) {
  boolean matched = false;
  for (Purchase purchase : booking.getPurchaseList()) {
    if (purchase.matchesType(typeToFind)) {
      matched = true;
      break;
    }
  }
  matched        // passes result to findAll, last-line-no-semicolon
}

So, adding the closure has made things worse. We had to introduce an artificial matched variable, and the result was returned on the last line without a semicolon, which is highly error prone and very un-Java-like.

In my prior blog post on this topic I suggested one possibility was using the goto keyword to help. But the truth is thats just horrible. So, I'll propose an alternative which I believe fits BGGA much better:

List[Booking] matches = findAll(Booking booking : bookingList) {
  for (Purchase purchase : booking.getPurchaseList()) {
    if (purchase.matchesType(typeToFind)) {
      => true;        // passes result to findAll of true 
    }
  }
  => false;           // passes result to findAll of false
}

So, this proposal is to simply use the => symbol to act as the result syntax within a closure. The true/false value is returned to the closure method which then decides whether to add the booking to the overall result or not. This fits the other use of => in the function type declaration, where it means 'returning'.

So, the overall effect is that a result can be returned from any part of the closure block, and there are no missing semicolons or weird last line effects.

Nested closures

Neal has previously raised a problem with this approach. He points out that the use of something like this means that refactoring the method further to introduce a second closure may cause problems. Here we change the inner for loop to the 'eachNotNull' closure:

List[Booking] matches = findAll(Booking booking : bookingList) {
  eachNotNull(Purchase purchase : booking.getPurchaseList()) {
    if (purchase.matchesType(typeToFind)) {
      => true;        // now within two closures! help!
    }
  }
  => false;           // passes result to findAll of false
}

As shown, the '=> true' is now within two closures, 'findAll' and 'eachNotNull'. So what does the => mean? Well, this actually isn't a new concept in Java - break and continue currently refer to the closest loop, and so are vulnerable to refactorings that introduce a new loop level. As such, I suspect that this isn't as big a deal as Neal thinks.

Option A for this situation is that the => operator returns the result to the closest surrounding closure that accepts that type of the result. In this case, the 'eachNotNull' closure has a 'Void' result, so the boolean value 'true' is still passed back to the 'findAll' closure. Thus, adding the 'eachNotNull' closure had no effect on the logic.

Option B is that any result within a second closure has to be labelled to reach its destination. This is the same rule as break/continue from a loop within a loop in current Java.

outer:   // label
List[Booking] matches = findAll(Booking booking : bookingList) {
  eachNotNull(Purchase purchase : booking.getPurchaseList()) {
    if (purchase.matchesType(typeToFind)) {
      => outer: true;    // labelled result now reaches findAll
    }
  }
  => false;              // no need to label, result goes to closest closure, findAll
}

Here, the closure is labelled 'outer' in exactly the same way as in current Java. The => result then references the label to return the result to the correct closure.

So which do you prefer? Option A (auto-seek best first matching result) or option B (explicit labelling)? I suspect that option A is too vague and complex for Java, so I'm leaning towards option B.

Fully refactored

By the way, if the code was refactored a little more, this is what you might end up with:

List[Booking] matches = findAll(Booking booking : bookingList) {
  => booking.matchesType(typeToFind);
}

To my eyes, this is the most attractive form, making it clear that a result is being returned from the closure block, and retaining the standard Java semicolon.

Summary

I'm determined to excise the evil 'last-line-no-semicolon' concept from closures - its a seriously nasty hack. Let me know your thoughts on it!