1
 public boolean sendDeviceEvent() {
    boolean status = false;
    try {
        device.sendEvent("blah...blah");
        status = true;
    } catch (Exception e) {
        log.error("Failed to send NodeLowBattery Event - {} {}", createNodeLowBatteryNotification(), e.getCause());
    } finally {
        return status;
    }
}

I would like to know how the above code can be considered bad practise since it returns from finally. Based on the byte code information, finally doesn't return abruptly and no values are set in finally. How can this be considered bad ?

bordatoue
  • 31
  • 1

1 Answers1

5

The point is: that finally statement simply doesn't make any sense. It adds no value whatsoever to your code. This version:

try {
 ...
 return true;
} catch (...) {
 log ...
}
return false;

does the very same; without making you start to think: what is that finally good for?

In other words: don't get too hang up on functionality; and forget about readability. You want to able to understand what is going on as quickly as possible. Using finally will definitely make your "brain cpu" spin "harder" ... just because you have to read it, and then digest and resolve to "ah, actually I don't need that at all".

Of course, this is very subtle; but in the end: a file full of subtle things that could be a bit clearer ... still makes up a file much harder to read than it ought to be!

Finally: don't get me wrong: there might be occasions where returning from the finally block could make sense. But - only when other things happen in that block (meaning there is a real need to have that finally block anyway).

GhostCat
  • 137,827
  • 25
  • 176
  • 248