9

I have a wrapper responsible for logging operations, named OperationWrapper. Its structure is simple and as follows:

public void runOperation(Operation o) throws Exception{ 
     logOperationStarted();
     o.execute();
     logOperationFinished();
}

Since the "o" operation can throw an exception, the logOperationFinished() method will not always be called and thus logging does not operate correctly.

Also, various components invoking the runOperation() method handle these exceptions .

Having wanted to ensure that logOperationFinished() will always run, I implemented the following structure:

public void runOperation(Operation o) throws Exception{ 
     logOperationStarted();
     try{
       o.execute();
     }
     catch(Exception e){
       throw e; 
     }
     finally{
       logOperationFinished();
     }
}

Now logOperationFinished() does always run, but I am getting a warning from IntelliJ:

Caught exception is immediately rethrown
Reports any catch block where the caught exception is immediately rethrown, without performing any action on it. Such catch blocks are unnecessary or lack error handling.

To me it seems the IntelliJ is not taking the finally block into account when issuing this warning.

Am I doing something wrong or is there a better way of achieving this?

Thanks.

Shahar
  • 478
  • 5
  • 17
  • Why are you catching it to begin with if you're only going to throw it again? It's not like you're doing anything with the exception directly. (Also, it's *poor* practice to catch `Exception` - avoid that where you can.) – Makoto Apr 07 '15 at 05:14

3 Answers3

10

yes, you do not need the catch

public void runOperation(Operation o) throws Exception{ 
     logOperationStarted();
     try{
       o.execute();
     }
     finally{
       logOperationFinished();
     }
}
Scary Wombat
  • 44,617
  • 6
  • 35
  • 64
  • Although this falls into Java basics, sometimes the shortest solution is the best. "You don't need the catch" is about as clear and concise as it can get. Sumit Singh's accepted solution may be valid, but yours is quick and to the point without the IF A THEN B logic. Thanks! – tresf Aug 28 '15 at 13:19
2

Is the exception meant to be recoverable? If not, it may be appropriate to throw an error, rather than rethrow the exception.

Scary Wombat's answer is most likely what you are after though.

throw new Error(e);

Exception vs Error

user2469515
  • 373
  • 3
  • 12
2

Use try-finally block from JLS 14.20.2. Execution of try-finally

If execution of the try block completes abruptly for any other reason R, then the finally block is executed, and then there is a choice:

  1. If the finally block completes normally, then the try statement completes abruptly for reason R.

  2. If the finally block completes abruptly for reason S, then the try statement completes abruptly for reason S (and reason R is discarded).

public void runOperation(Operation o) throws Exception{ 
     logOperationStarted();
     try{
        o.execute();
     }finally{
       logOperationFinished();
     }
}

If you want to use try-catch-finally still not a problem you can ignore warning from IntelliJ, or throw new exception from catch block.

throw new ExceptionYouWantToThrow(e);
Community
  • 1
  • 1
Sumit Singh
  • 15,743
  • 6
  • 59
  • 89