Is this an abuse of try/finally?

Question

Given that multiple return statements are acceptable (I disagree, but let us digress), I'm looking for a more acceptable way to achieve the following behavior:

Option A: multiple returns, repeated code block

public bool myMethod() {
    /* ... code ... */

    if(thisCondition) {
        /* ... code that must run at end of method ... */
        return false;
    }

    /* ... more code ... */

    if(thatCondition) {
        /* ... the SAME code that must run at end of method ... */
        return false;
    }

    /* ... even more code ... */

    /* ... the SAME CODE AGAIN that must run at end of method ... */
    return lastCondition;
}

It makes me feel dirty to see the same (little) code block repeated three times each time the method returns. Furthermore, I would like to clarify that the two return false statements above can certainly be described as returning mid-method... they are absolutely not "guard statements."

Is Option B slightly more acceptable? I feel that I may abusing try/finally, and I'm hoping there is something completely different that I should be doing.

Option B: multiple returns, try/finally block (without catch blocks / exceptions)

public bool myMethod() {
    try {
        /* ... code ... */

        if(thisCondition) {
            return false;
        }

        /* ... more code ... */

        if(thatCondition) {
            return false;
        }

        /* ... even more code ... */

        return lastCondition;
    } finally {
        /* ... code that must run at end of method ... */
    }
}

Finally, Option C is the best solution in my book, but my team doesn't like this approach for whatever reason(s), hence I'm looking for a compromise.

Option C: single return, conditional blocks

public bool myMethod() {
    /* ... code ... */

    if(!thisCondition) {
        /* ... more code ... */
    }

    if(!thisCondition && !thatCondition) {
        /* ... even more code ... */
    }

    /* ... code that must run at end of method ... */
    return summaryCondition;
}

If you want to discuss multiple return statements, please do so in this question.

Answer 1

If the code needs to run even when any other code throws an exception, then the finally block is the correct solution.

If it need not run in the case of an exception (i.e. it's only necessary for "normal" returns), then using finally would be abusing the feature.

Personally I'd rewrite that method in the single-return-point style. Not because I religiously subscribe to that idea (I don't), but because it is best suited for these kind of end-of-method codes.

When that code turns out to be overly complicated (and that's a very real possibility), then it's time to refactor the method by extracting one or more methods.

The simplest refactoring would be something like this:

public boolean  myMethod() {
    boolean result = myExtractedMethod();
    /* ... code that must run at end of method ... */
    return result;
}

protected boolean myExtractedMethod() {
    /* ... code ... */

    if(thisCondition) {
        return false;
    }

    /* ... more code ... */

    if(thatCondition) {
        return false;
    }

    /* ... even more code ... */
    return lastCondition;
}

Answer 2

Exceptions should be exceptional so I don't like option B if there are no other exceptions around

If code is always needed how about refactoring into 2 functions

public bool myMethod() {
    bool summaryCondition = myMethodWork();
    // do common code
    return summaryCondition;
}

private bool myMethodWork() {
   /* ... code ... */

    if(thisCondition) {
        return false;
    }

    /* ... more code ... */

    if(thatCondition) {
        return false;
    }

    /* ... even more code ... */

    return lastCondition;
}

Answer 3

Using try/finally for controlling flow feels to me like using GOTO.

Answer 4

Is there a reason that you can't simply store the return value, and fall out of the if?

   bool retVal = true;
   if (retVal && thisCondition) {
   }

   /* more code */

   if ( retVal ) {
     /* ... code that must run at end of method, maybe inside an if or maybe not... */

   }
   return retVal;

Answer 5

IMO the idea is to put a try block over a small part of code (e.g. a method call) that can throw a known exception (e.g. reading from a file, reading an int to a String). So putting a try block over the whole code of a method is really not the way to go, unless you are actually expecting each and every if condition code to potentially throw the same set of exceptions. I don't see a point in using a try block just for the sake of using a finally.

If I am not mistaken, putting large chunks of code inside a try also makes it a lot slower, but not sure if this is true in the latest versions of Java.

Personally, I would go with Option C. But I don't have anything against Option A either.

Answer 6

This is a perfect place for a GOTO

*ducks*

Answer 7

Unless the code that must run at the end of the method uses method local variables, you could extract it into a method like:

public boolean myMethod() {
    /* ... code ... */

    if(thisCondition) {
        return myMethodCleanup(false);
    }

    /* ... more code ... */

    if(thatCondition) {
        return myMethodCleanup(false);
    }

    /* ... even more code ... */

    return myMethodCleanup(lastCondition);
}

private boolean myMethodCleanup(boolean result) {

    /* ... the CODE that must run at end of method ... */
    return result;
}

this still doesn't look very nice, but it is better than using goto-like constructs. To convince your team mates that a 1-return solution might not be that bad, you can also present a version using 2 do { ... } while (false); and break's (*evil grin*.)

Answer 8

How about breaking it up a little more to give something more ( excusing my not having used Java's logical operators in quite some time ) like this:

public bool findFirstCondition()
{
   // do some stuff giving the return value of the original "thisCondition".
}

public bool findSecondCondition()
{
   // do some stuff giving the return value of the original "thatCondition".
}

public bool findLastCondition()
{
   // do some stuff giving the return value of the original "lastCondition".
}

private void cleanUp() 
{
   // perform common cleanup tasks.
}


public bool myMethod() 
{ 


   bool returnval = true;
   returnval = returnval && findFirstCondition();
   returnval = returnval && findSecondCondition();

   returnval = returnval && findLastCondition();
   cleanUp();
   return returnval; 
}

Answer 9

If the code needs to run even when there is an Exception, then finally is not just a good choice, it is a must. If that is not the case, finally is not necessary. Looks like you want to find format that "looks" best. But there is little more at stake here.

Answer 10

Don't abuse try/finally unless you need to break out of inner loops. Abuse do/while.

bool result = false;
do {
  // Code
  if (condition1) break;
  // Code
  if (condition2) break;
  // . . .
  result = lastCondition
} while (false);

Answer 11

Your option C solution is not far from optimal, since it adequately codes the proper execution sequence you're trying to accomplish.

Similarly, using nested if-statements do the same thing. It may be visually less appealing, but it's simpler to understand, and makes the execution flow pretty obvious:

public bool myMethod() { 
    boolean  rc = lastCondition; 

    /* ... code-1 ... */ 

    if (thisCondition) { 
        rc = false;
    } 
    else {  
        /* ... code-2 ... */ 

        if (thatCondition) { 
            rc = false;
        } 
        else {  
            /* ... code-3 ... */ 
            rc = ???;
        }  
    }

    /* ... the code that must run at end of method ... */ 
    return rc;  
}

Simplifying the code yields:

public bool myMethod() { 
    boolean  rc = false; 

    /* ... code-1 ... */ 

    if (!thisCondition) { 
        /* ... code-2 ... */ 

        if (!thatCondition) { 
            /* ... code-3 ... */ 
            rc = lastCondition;
        }  
    }

    /* ... the code that must run at end of method ... */ 
    return rc;  
}

The simplified code also reveals what you're actually trying to achieve: you are using the test conditions to avoid executing code, therefore you should probably be executing that code when the conditions are false instead of doing something when they are true.

To answer your question about try-finally blocks: Yes, you can abuse them. Your example is not sufficiently complex enough to warrant the use of try-finally. If it were more complex, though, it might.

See my take on it at: Go To Statement Considered Harmful: A Retrospective, "Exception Handling".

Answer 12

Can someone please explain to me why a the use of a Switch-Case wouldn't be optimal here?