8

I have a try block where database queries are attempted to be run and a finally block where database resources are released. If a value does not exist in the database I return null.

Is it a good idea to return in a try block?

Some example code:

    try {
        if (!jedis.exists(name)) {
            return null; // Is this a good idea?
        }

        // Do database related stuff...
    } catch (JedisConnectionException e) {
        // Fix any problems that happen
    } finally {
        // Return all objects to pools and clean up
    }
user2248702
  • 2,741
  • 7
  • 41
  • 69
  • why dont you use [try with resource](http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html) – SpringLearner Jul 23 '14 at 11:11
  • Even if you use return statement, the finally block would be executed. – ngrashia Jul 23 '14 at 11:12
  • There's no problem in returning from a try block. – JB Nizet Jul 23 '14 at 11:12
  • there's nothing wrong with it, if it's a short piece of code with a single return. Multiple returns will make it harder to refactor your code if you want to extract snippets into new methods. Notice that the finally clause will still be executed. – Leo Jul 23 '14 at 11:13
  • 2
    "Is it a good idea to return in a try block" what makes you think it *might* be a bad idea? – Raedwald Jul 23 '14 at 11:20
  • Are you, perhaps, concerned that your `finally` clause might not run? If so, this question is a duplicate of http://stackoverflow.com/questions/65035/does-finally-always-execute-in-java?rq=1 – Raedwald Jul 23 '14 at 11:21
  • I think it is a bad idea to return a value from the Finally block. Other than that every thing is OK – naveejr Jul 23 '14 at 11:37

7 Answers7

13

Is it a good idea to return in a try block?

Absolutely: if the preparation of an object to be returned fits entirely within the scope of the try block, there is no reason to extend its visibility beyond its natural boundaries.

As an illustration, this

try {
    ReturnType ret = ...
    // Compute ret
    return ret;
} catch {
    ...
    // need to either throw or return something
} finally {
    ...
}

is better than this

ReturnType ret = ...
try {
    // Compute ret
} catch {
    ...
} finally {
    ...
}
return ret;
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • 2
    It will make a compilation Error. You should return at least a value inside the Catch Block. In case of exception before return in the try block the method should return some value. – naveejr Jul 23 '14 at 11:30
  • I think if there is not just the try but also the finally block is present, it's logically not right to return from the try block because the finally block will execute after you returned from the try block regardless. I strongly believe that a method should not do anything after it has returned. – sasieightynine Apr 29 '22 at 15:35
  • @sasieightynine As long as the `finally` block not touching the `ret` variable, it does not matter if it's there or not. Doing some cleanup outside the "main path" is a good thing. – Sergey Kalinichenko Apr 29 '22 at 15:40
2

In general there is nothing wrong with returning from a try-block. However it is good practice to keep blocks short, so having irrelevant code within a try block is not great.

Maybe the following is an improvement in your case:

// assuming a String result type for sake of demonstration
String result = null;
if (jedis.exists(name)) {        
    try {
        result = jedis.getResult(name);
    } catch (JedisConnectionException e) {       
        LOG.error("Could not get result", e);
    } finally {
        jedis.close();
    }
}
return result;

You probably cannot 'fix' the JedisConnectionException so I would either log (as shown above) or rethrow:

throw new MyAppException("Could not get result", e);

Don't log AND rethrow, it will give you and others headaches.

As mentioned you can also use try-with-resources in Java SE 7 if JedisConnection is a Closable:

    try (JedisConnection jedis = Pool.getJedisConnection()) {
        result = jedis.getResult(name);
    } catch (JedisConnectionException e) {       
        LOG.error("Could not get result", e);
    } 

This will automatically close the JedisConnection after processing is done.

Adriaan Koster
  • 15,870
  • 5
  • 45
  • 60
1

Consider this example. finally block will be executed even if you return in the try block. Its better to return in the finally block for better readability of code . You can trace from where your value is being returned, else you might end up with some problems figuring that out.

public static void main(String[] args) {
        System.out.println(someMethod());
    }

    private static boolean someMethod() {
        try {
            System.out.println("in try");
            return true;
        } catch (Exception e) {

        } finally {
            System.out.println("in finally");
            return false;
        }

    }

O/P :

in try
in finally
false -- > not true, but false
TheLostMind
  • 35,966
  • 12
  • 68
  • 104
0

Even if you use return statement, the finally block would be executed.

The finally block would not be executed only when you call System.exit(0);

So, you can use your clean up code in finally block.

Using return however depends on other stuff like your solution design.

ngrashia
  • 9,869
  • 5
  • 43
  • 58
0

As long as finally really has to run even if !jedis.exists(name) evaluates to true, it's not only a good idea, it's practically a must. So for example, if you have a line like jedis.connect() (not real code, just an example), then the if should be inside the try, and finally should call jedis.disconnect. But if the code in your finally is fully dependent on what you would do after the check, then the if makes more sense before the try.

Selali Adobor
  • 2,060
  • 18
  • 30
0

That depends, in your case it doesn´t matter, but as a general rule, consider this example:

String returnString="this will be returned";
try
{
    // ... something that could throw SomeException
    return returnString;
}
catch(SomeException ex)
{
    // Exception handling
}
finally
{
   returnString="this will not be returned";
}

This might be somehow confusing, since the string returned is "this will be returned", no matter what finally was trying to do.

0

It is OK to reaturn a value from the try block.

Consider this case also in your implementation.

private static int testFinally() {
        try {
            return 100;
        } finally {
            return 101;
        }
    }

here the values are returned from try block and finally block. First it will execute the try block but before return it should execute the finally block. So this method will return 101, from the finally block instead of 100.

So returning from the finally block might produce unexpected results for the casual reader.

In my view

It is better to return values inside the Try block

Return the Default values/ appropriate values inside the Catch blocks in case of exceptions.

Only do the cleanups inside the finally block.

naveejr
  • 735
  • 1
  • 15
  • 31