8

I'm working on a legacy system that has a custom exception that's used gosh-frickity-everywhere. It was inspired by the ServletException class, that said "well any time you have an exception in your Servlet, you'll want to throw this ServletException".

As the system evolved (over 10 years) a more robust system of catching the exceptions at a higher level has taken place and it's no longer necessary to wrap every exception in this custom exception. (One could argue it never was, but that's another story. It's a stable app, so I tend not to complain too much!!) But we're not going to be refactoring them all at once, just slowly over time.

However, one thing that would make things simpler going forward would be if the custom exception were a runtime exception instead of a checked exception. This way we wouldn't need to explicitly catch it everywhere, and legacy code that hasn't been refactored yet will just continue to throw this the same way as they'd throw a null pointer exception if one occurred.

My question is... What are the side effects of taking a once checked exception and making it a runtime exception?

I can't think of any, aside from warnings for unnecessary check and throws declarations, but it would be nice to get input from someone who has been down this road before.

corsiKa
  • 81,495
  • 25
  • 153
  • 204
  • 2
    Excellent use of, "gosh-frickity-everywhere" – Gareth Aug 29 '14 at 14:45
  • 3
    It's an elegant weapon from a more civilized age. – corsiKa Aug 29 '14 at 14:46
  • Like you, I can't really think of any side effects to this in the immediate term. For the longer term, you might need to think about what handling needs to happen if this exception is propagated all the way up the stack chain. – ControlAltDel Aug 29 '14 at 14:46
  • The fact that we can handle any Exception, not just our custom exception, as it propagates up the stack is why I'd like to phase out the custom exception. But since we'll phase it out over time, we can simplify new code that uses old code by making it a runtime exception. – corsiKa Aug 29 '14 at 14:48

3 Answers3

2

Changing a checked exception into an unchecked one has very little practical effect on existing, working code, but you do need to watch out for the possibility that somewhere in your code you catch (RuntimeException ...). Such catches do not intercept your custom exception now, but they would do if you make it unchecked.

You might also run into issues if you do anything reflective related to methods that throw that exception (which apparently is most of them).

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
  • Thankfully we do very little reflection. A little bit of bean population but that's it. As for catching RuntimeException that's a good point - I did a codebase search and found 7 instances and they are all clear. Either they also catch the custom exception or otherwise have the same behavior. That's a good point though, it could cause a different execution path. – corsiKa Aug 29 '14 at 15:06
0

Something like that happened on an old module of a app that I had to maintain. The only problem translating exceptions to runtime is that you may lose granularity but that is entirely up to you to handle.

For example, we had a ton of code like this in the deeper layers:

catch(IOException e){    
    Throwables.propagate(e);
}

We used that pattern carelessly all over that layer and, when we needed to check the exception cause, we always had to get the cause of the exception and that made a lot of boilerplate in higher layers. To this day I believe it's better to create a good class hierarchy of non-checked exceptions in order to preserve granularity. e.g.

catch(IOException e){
    throw new FileNotCreatedException(e);
}

And with this you can catch the exception easily in other layers and divide errors and fallbacks easily:

catch(FileNotCreatedException e){
   notifyError(e);
} catch(NoMoreStorageException e){
   releaseSomeStorage();
   retryOperation();
}
ElderMael
  • 7,000
  • 5
  • 34
  • 53
  • Since the person is keeping the same class name, I'm not sure how this answer applies. – JustinKSU Aug 29 '14 at 14:59
  • Actually we gain granularity as we phase out the custom exception that wraps everything and more and more use exceptions that actually describe what the problem is. – corsiKa Aug 29 '14 at 15:04
0

Here are a few that I can think of

  • Runtime exceptions could potentially go to a layer where you didnot intend it to go.
    • ex: a Servlet ---> Service ---> DAO. Runtime exceptions thrown by DAO related to database interactions could potentially land up on Servlet. Clear segregation of layers where each layer handles all exceptions (checked/runtime) at its entry points can ensure that this doesn't happen.
  • Runtime exceptions can leave system in an inconsistent state.
    • ex: If the sequence diagram looks like Class A --> Class B ---> Class C and if Class B1 is injected between B and C therefore Class A ---> Class B ---> Class B1 ---> Class C then neither Class A, B, B1 would know that they might have to cleanup when this runtime exception occurs in Class C. In fact this can potentially affect the semantics of any dependency injection.

As a general thumb rule in my opinion:

  • Use checked exceptions when you "expect" the exception as part of a normal control flow and know how to handle it. ex: Validation of a business rule say account balance has to be greater than debit amount by atleast 100.
  • Use unchecked exceptions when you "don't expect" the exception as part of a normal control flow hence have no means of handling it yet you know that "some class" within your "layer" would handle it to ensure graceful degradation ex: DB connection lost would have handled by your "service" layer entry class.
  • Never use error. Only JRE has reasons to throw errors.
sandeepkunkunuru
  • 6,150
  • 5
  • 33
  • 37
  • Ironically, the scenario you describe for the first potential problem is exactly why we want to replace it. Right now, all exceptions basically get wrapped and just thrown up to a filter that checks for the custom exception. I'd like to keep the exceptions at the lower layers if possible. But I see how what you describe could happen if it wasn't happening already! :) – corsiKa Aug 29 '14 at 15:27