2

I have following code using hibernate to throw a custom exception on error and I also want to close the session in this case, since the exception won't be catched unless received on the client machine.

public <T> T get(final Session session, final String queryName) throws RemoteException
{
    final Query query = // query using given session ...

    try
    {
        return (T) query.uniqueResult();
    }
    catch (final HibernateException e)
    {
        SessionManager.logger.log(Level.SEVERE, "Could not retrieve Data", e);
        this.closeSession(session);
        throw new RemoteException("Could not retrieve Data");
    }
}

Now I have a helper method which closes the session and throws a given exception:

public void closeSessionAndThrow(final Session session, final RemoteException remoteException)
    throws RemoteException
{
    this.closeSession(session);
    throw remoteException;
}

Now I thought I could simplify my above code using:

public <T> T get(final Session session, final String queryName) throws RemoteException
{
    final Query query = // query using given session ...

    try
    {
        return (T) query.uniqueResult();
    }
    catch (final HibernateException e)
    {
        SessionManager.logger.log(Level.SEVERE, "Could not retrieve Data", e);
        this.closeSessionAndThrow(session, new RemoteException("Could not retrieve Data"));
    }
}

Now I need to add a return null; statement after the catch. Why?

djmj
  • 5,579
  • 5
  • 54
  • 92

5 Answers5

5

This is the problem with methods like closeSessionAndThrow. The JLS rules do not allow the compiler to infer that since the method unconditionally throws an exception it can never return normally. The calling code therefore has to be written as if the method could return ... even though "we know" it cannot happen.

You simply have to go with the flow. This is an "unusual" control pattern that the Java language doesn't support as well as you / we would like.


(Under certain circumstances, one can prove that the method will always throw an exception under certain assumptions. However, since the method is public and in a different class, one of the assumptions is that the classes that define and use the method won't be changed and compiled independently of each other. Of course, that is not the kind of assumption that a compiler can make ... and it partly explains why the JLS rules don't attempt to cover this pattern.

If they were to "fix" this problem, it would require something like an annotation, or change to the java syntax to declare that the helper method cannot return.)

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • But i like the provided answer the most. Its guaranteed and I can also remove my finally blocks in which I still had to close the session in other methods (moved the session close to try block), which reduced code a bit. – djmj Jun 26 '12 at 00:50
  • Yea ... it neatly sidesteps the problem. However, it doesn't work in all cases. For instance, suppose your helper method returned either `FileNotFoundException` or `InterruptedException`; i.e. two unrelated checked exceptions. Now the calling method has to be declared as throwing `Exception`. Bad, bad, bad. – Stephen C Jun 26 '12 at 00:57
  • @StephenC - This is another reason to favor unchecked exceptions. – David Harkness Jun 26 '12 at 02:11
  • @DavidHarkness - that doesn't help if you are using standard Java APIs that throw standard exceptions. Lets stick to reality ... – Stephen C Jun 26 '12 at 02:14
  • @StephenC - While the example uses `RemoteException`, the question itself is generic. When writing custom helper methods, choosing the exception type is hardly outside the bounds of reality. – David Harkness Jun 26 '12 at 02:20
  • @DavidHarkness - Umm ... so you are suggesting that people should routinely map standard checked exceptions to application specific unchecked ones? That does NOT strike me as good practice or good advice. – Stephen C Jun 26 '12 at 04:18
  • @StephenC - See [Java theory and practice: The exceptions debate](http://www.ibm.com/developerworks/java/library/j-jtp05254/index.html) for a good overview of this debate, specifically items 41 and 43 quoted from [Effective Java](http://www.amazon.com/Effective-Java-Edition-Joshua-Bloch/dp/0321356683). – David Harkness Jun 26 '12 at 04:45
4

Change the declaration of closeSessionAndThrow to return RemoteException and then "throw" the return result of calling it in your client code.

public RemoteException closeSessionAndThrow( ... )   // <-- add return type here
        throws RemoteException { ... }

public <T> T get( ... ) throws RemoteException
{
    try { ... }
    catch (final HibernateException e)
    {
        throw this.closeSessionAndThrow( ... );  // <-- add "throw" here
    }
}

This tricks the compiler into thinking it will always throw whatever exception is returned from closeSessionAndThrow. Since the helper method throws that exception itself, this second throw never comes into play. While you could return the exception from the helper, this invites error when someone forgets to add throw before the call.

David Harkness
  • 35,992
  • 10
  • 112
  • 134
  • Thanks for that answer. Yes i forget that the method is not guaranteed to throw that exception!, How should the compiler know this. – djmj Jun 26 '12 at 00:32
1

While closeSessionAndThrow always throws the remote exception, the compiler doesn't dive into the method to find that out, therefore the compiler only knows that it COULD throw a RemoteException, not that it WILL. I would maybe do something like this:

public RemoteException closeSessionAndThrow(final Session session, final Exception e, String msg)
{
    SessionManager.logger.log(Level.SEVERE, msg, e);
    this.closeSession(session);
    return new RemoteException(msg);
}

public <T> T get(final Session session, final String queryName) throws RemoteException
{
    final Query query = // query using given session ...

    try
    {
        return (T) query.uniqueResult();
    }
    catch (final HibernateException e)
    {
        throw this.closeSessionAndThrow(session, e, "Could not retrieve Data");
    }
}
LINEMAN78
  • 2,562
  • 16
  • 19
0

Since this method must return something, you should put a return null; after

this.closeSessionAndThrow(session, new RemoteException("Could not retrieve Data"));

but still within the catch clause or use a finally clause.

Why do you need to do this? Because your function must return something. and since one it returns from:

this.closeSessionAndThrow(session, new RemoteException("Could not retrieve Data"));

and doesn't return anything, this causes a compiler error.

Zéychin
  • 4,135
  • 2
  • 28
  • 27
-3

add finally to try-catch block finally { return null; } - If execution goes into try block and result is success it would return, so finally would never get executed. - If execution goes into catch block, the finally block gets executed after catch and would return null. Safe execution!

Not correct answer!

exception
  • 955
  • 2
  • 11
  • 23
  • Finally always gets executed, even on return statement. http://stackoverflow.com/questions/65035/in-java-does-return-trump-finally – djmj Jun 26 '12 at 00:30
  • Ouch - this will cause the code to ALWAYS return null, even in the case where there was no exception thrown or caught. That is NOT what the problem requires. – Stephen C Jun 26 '12 at 00:32
  • `Returning` from `finally` is always a *bad idea*. Do a Google search for the phrase and then delete or modify the answer :) – Miserable Variable Jun 26 '12 at 00:33