As part of writing an Iterator, I found myself writing the following piece of code (stripping error handling)

public T next() {
  try {
    return next;
  } finally {
    next = fetcher.fetchNext(next);

finding it slightly easier to read than

public T next() {
  T tmp = next;
  next = fetcher.fetchNext(next);
  return tmp;

I know it's a simple example, where the difference in readability may not be that overwhelming, but I'm interested in the general opinion as to whether it is bad to use try-finally in cases like this where there are no exceptions involved, or if it is actually preferred when it simplifies the code.

If it's bad: why? Style, performance, pitfalls, ...?

Conclusion Thanks you all your answers! I guess the conclusion (at least for me) is that the first example might have been more readable if it was a common pattern, but that it isn't. Therefore the confusion introduced by using a construct outside of it's purpose, along with possibly obfuscated exception flow, will outweigh any simplifications.

  • 10
    I personally would be able to understand the second one more than the first, but I seldom use finally blocks. Commented Apr 28, 2012 at 2:03
  • 2
    return current = fetcher.fetchNext(current); // How about this, aka do you really need prefetching?
    – scarfridge
    Commented Apr 28, 2012 at 8:33
  • 1
    @scarfridge The example is about implementing Iterator, where you indeed need sort of prefetching in order for hasNext() to work. Try it yourself.
    – maaartinus
    Commented Apr 28, 2012 at 12:37
  • 1
    @maaartinus I'm aware of that. Sometimes hasNext() simply returns true e.g. hailstone sequence and sometimes fetching the next element is prohibitively expensive. In the latter case you should resort to fetching the element only on demand similar to lazy initialization.
    – scarfridge
    Commented Apr 28, 2012 at 16:52
  • 1
    @scarfridge The problem with common use of Iterator is that you need to fetch the value in hasNext() ('cause fetching it is quite often the only way to find out if it exists) and return it in next() just like the OP did.
    – maaartinus
    Commented Apr 28, 2012 at 19:07

5 Answers 5


Personally, I'd give a thought to the idea of command query separation. When you get right down to it, next() has two purposes: the advertised purpose of retrieving the element next, and the hidden side effect of mutating the internal state of next. So, what you're doing is performing the advertised purpose in the body of the method and then tacking on a hidden side effect in a finally clause, which seems... awkward, though not 'wrong', exactly.

What it really boils down to is how understandable the code is, I'd say, and in this case the answer is "meh". You're "trying" a simple return statement and then executing a hidden side effect in a code block that's supposed to be for fail-safe error recovery. What you're doing is 'clever', and 'clever' code often induces maintainers to mumble, "what the... oh, I think I get it." Better for people reading your code to mumble "yep, uh-huh, makes sense, yes..."

So, what if you separated state mutation from accessor calls? I'd imagine the readability issue you're concerned with becomes moot, but I don't know how that affects your broader abstraction. Something to consider, anyway.

  • 1
    +1 for the "clever" comment. That very accurately captures this example.
    – user53019
    Commented May 3, 2012 at 18:30
  • 3
    The 'return and advance' action of next() is forced upon the OP by the unwieldy Java iterator interface. Commented May 6, 2012 at 17:16

Purely from a style standpoint, I think these three lines:

T current = next;
next = fetcher.getNext();
return current;

... are both more obvious and shorter than a try/finally block. Since you're not expecting any exceptions to be thrown, using a try block is just going to confuse people.

  • 2
    A try/catch/finally block might also add extra overhead, I'm not sure if javac or the JIT compiler would strip them out even if you're not throwing an Exception. Commented Apr 28, 2012 at 10:32
  • 2
    @MartijnVerburg yeah, javac still adds an entry to the exception table and duplicates the finally code even if the try block is empty. Commented Apr 28, 2012 at 12:28
  • @Daniel - thanks I was too lazy to execute javap :-) Commented Apr 28, 2012 at 12:48
  • @Martijn Verburg: AFAIK, a try-catch-finallt block is essentially free as long as there's no actual exception thrown. Here, javac doesn't try and doesn't need to optimize anything as JIT will take care of it.
    – maaartinus
    Commented Nov 13, 2012 at 15:02
  • @maaartinus - thanks that does make sense - need to read OpenJDK source code again :-) Commented Nov 13, 2012 at 15:30

That would depend on the purpose of the code inside the finally block. The canonical example is closing a stream after reading/writing from it, some sort of "cleanup" that must be always done. Restoring an object (in this case an iterator) to a valid state IMHO also counts as cleanup, so I see no problem here. If OTOH you were using return as soon as your return value was found, and adding a lot of unrelated code in the finally block, then it would obscure the purpose and make it all less understandable.

I don't see any problem in using it when "there are no exceptions involved". It's very common to use try...finally without a catch when the code can only throw RuntimeException and you don't plan on handling them. Sometimes, the finally is just a safeguard, and you know for the logic of your program that no exception will ever be thrown (the classic "this should never happen" condition).

Pitfalls: any exception raised inside the try block will make the finally bock run. That can put you in an inconsistent state. So, if your return statement were something like:

return preProcess(next);

and this code raised an exception, fetchNext would still run. OTOH if you coded it like:

T ret = preProcess(next);
next = fetcher.fetchNext(next);
return ret;

then it would not run. I know you're assuming the try code can never raise any exception, but for cases more complex than this how can you be sure? If your iterator were a long-lived object, that would continue existing even if an unrecoverable error happened in the current thread, than it would be important to keep it in a valid state at all times. Otherwise, it doesn't really matter much...

Performance: it would be interesting to decompile such code to see how it works under the hood, but I don't know enough of the JVM to even take a good guess... Exceptions are usually "exceptional", so the code that handles them don't need to be optimized for speed (hence the advice to never use exceptions in the normal control flow of your programs), but I dunno about finally.

  • Aren't you actually providing a pretty good reason to avoid using finally in this way, then? I mean, as you say, the code ends up having unwanted consequences; even worse, you are probably not even thinking about exceptions.
    – Andres F.
    Commented Apr 28, 2012 at 3:07
  • 2
    The normal control flow speed is not significantly affected by the exception-handling constructs. The performance penalty is only paid when actually throwing and catching exceptions, not when entering a try block or entering a finally block in normal operation.
    – yfeldblum
    Commented Apr 28, 2012 at 3:17
  • 1
    @AndresF. True, but that's also valid for any cleanup code. For example, suppose a reference to an open stream is set to null by a buggy code; trying to read from it will raise an exception, trying to close it in the finally block will raise another exception. Besides, this is a situation where the state of the computation doesn't matter, you want to close the stream whether or not an exception occurred. There might be other situations like that as well. For this reason I'm treating it as a pitfall for the valid uses, rather than a reccomendation to never use it.
    – mgibsonbr
    Commented Apr 28, 2012 at 3:22
  • There is also the problem that when both the try and finally block throw an exception, the exception in the try is never seen by anyone, but it is probably the cause of the problem. Commented Apr 30, 2012 at 20:18

The title statement: "...finally clause for doing work after return..." is false. The finally block happens before the function returns. That is the whole point of finally in fact.

What you are abusing here is the order of evaluation, where the value of next is stored for returning before you mutate it. It is not common practice and in my opinion wrong as it makes you code non-sequential and therefore much harder to follow.

The intended use of finally blocks is for exception handling where some consequence must happen regardless of exceptions being thrown e.g. cleaning up some resource, closing DB connection, closing a file/socket etc.

  • +1, I would add that even if the language spec guarantees that the value is stored before it is returned, it pitentially goes against reader expectation, and thus should be avoided when reasonably possible. Debugging is twice as hard as coding...
    – jmoreno
    Commented Jan 11, 2013 at 9:01

I would be really carefull, because (in general, not in your example) part in try can throw exception, and if it is thrown out, nothing will be returned and if you actualy inteded to execute what is in finally after return, it will execute when you did not want it to.

And other can of worm is, that in general, some usefull action in finally can throw exceptions, so you can end with really messy method.

Because of these, someone reading it must think about these problems, so it is harder to understand.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.