Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I read some code of a colleague and found that he often catches various exceptions and then always throws a 'RuntimeException' instead. I always thought this is very bad practice. Am I wrong?

share|improve this question
add comment

9 Answers

I do not know enough context to know whether your colleague is doing something incorrectly or not, so I am going to argue about this in a general sense.

I do not think it is always an incorrect practice to turn checked exceptions into some flavor of runtime exception. Checked exceptions are often misused and abused by developers.

It is very easy to use checked exceptions when they are not meant to be used (unrecoverable conditions, or even control flow). Especially if a checked exception is used for conditions from which the caller cannot recover, I think it is justified to turn that exception to a runtime exception with a helpful message/state. Unfortunately in many cases when one is faced with an unrecoverable condition, they tend to have an empty catch block which is one of the worst things you can do. Debugging such an issue is one of the biggest pains a developer can encounter.

So if you think that you are dealing with a recoverable condition, it should be handled accordingly and the exception should not be turned into a runtime exception. If a checked exception is used for unrecoverable conditions, turning it into a runtime exception is justified.

share|improve this answer
    
In most real-life applications, there are very few unrecoverable conditions. There's nearly alwas some level at which you can and should say "OK, this action failed, so we show/log a nice error message and continue with / wait for the next one". –  Michael Borgwardt Nov 24 '11 at 13:31
add comment

No, you are not wrong. His practice is extremely misguided. You should throw an exception that captures the issue that caused it. RunTimeException is broad and over reaching. It should be a NullPointerException, ArgumentException, etc. Whatever accurately describes what went wrong. This provides the ability to differentiate issues that you should handle and let the program survive versus errors that should be a "Do not pass go" scenario. What he is doing is only slightly better than "On Error Resume Next" unless there is something missing in the info provided in the question.

share|improve this answer
18  
@Gary Buyn: many people think that checked exception are a failed language design experiment and they are the ones that should be used sparingly, not as a matter of habit. –  Michael Borgwardt Nov 23 '11 at 20:17
4  
@Gary Buyn: Here's an article that outlines the debate pretty well: ibm.com/developerworks/java/library/j-jtp05254/index.html Note also that over 15 years after Java introduced this feature, no other language has adopted it, and C++ has deprecated a similar feature. –  Michael Borgwardt Nov 23 '11 at 20:56
6  
@c_maker: Actually, Bloch mostly promotes the orthodox view, and your comment seems to be mainly about using more exceptions, period. My view is that the only valid reason to use a checked exception is for a condition that you expect all callers to handle immediately. –  Michael Borgwardt Nov 24 '11 at 13:26
1  
Prove it. I have never ever seen any guidelines that say catch an exception and throw some generic exception with no meaning. You can wrap an exception and propagate something meaningful but the OP didn't say that is what is happening. He says he is just tossing a random runtime exception which will make it a living hell for someone to debug down the road. –  Rig Jan 9 '12 at 22:58
9  
Unnecessary throwing checked exceptions violates encapsulation. What do you do, if a simple method like 'getAccounts()' throws you an 'SQLException', 'NullPointerException' or 'FileNotFoundException'? Can you handle it? Youll probably just 'catch(Exception e){}' it. Besides, those exceptions - its implementation specific! It should not be a part of the contract! All you need to know is that there was an error. And what if the implementation changes? Suddenly everything has to change, cause the method no longer throws 'SQLException', but an 'ParseXMLException' instead! –  K.L. Nov 15 '12 at 15:32
show 8 more comments

It can be GOOD. Please read:

http://onjava.com/pub/a/onjava/2003/11/19/exceptions.html

Most of the time, client code cannot do anything about SQLExceptions. Do not hesitate to convert them into unchecked exceptions. Consider the following piece of code:

public void dataAccessCode(){
  try{
      ..some code that throws SQLException
  }catch(SQLException ex){
      ex.printStacktrace();
  }
} 

This catch block just suppresses the exception and does nothing. The justification is that there is nothing my client could do about an SQLException. How about dealing with it in the following manner?

public void dataAccessCode(){
   try{
       ..some code that throws SQLException
   }catch(SQLException ex){
       throw new RuntimeException(ex);
   }
} 

This converts SQLException to RuntimeException. If SQLException occurs, the catch clause throws a new RuntimeException. The execution thread is suspended and the exception gets reported. However, I am not corrupting my business object layer with unnecessary exception handling, especially since it cannot do anything about an SQLException. If my catch needs the root exception cause, I can make use of the getCause() method available in all exception classes as of JDK1.4.

Throwing checked exceptions and not being able to recover from it is not helping.

Some people even think that checked exceptions should not be used at all. See http://www.ibm.com/developerworks/java/library/j-jtp05254/index.html

Recently, several well-regarded experts, including Bruce Eckel and Rod Johnson, have publicly stated that while they initially agreed completely with the orthodox position on checked exceptions, they've concluded that exclusive use of checked exceptions is not as good an idea as it appeared at first, and that checked exceptions have become a significant source of problems for many large projects. Eckel takes a more extreme view, suggesting that all exceptions should be unchecked; Johnson's view is more conservative, but still suggests that the orthodox preference for checked exceptions is excessive. (It's worth noting that the architects of C#, who almost certainly had plenty of experience using Java technology, chose to omit checked exceptions from the language design, making all exceptions unchecked exceptions. They did, however, leave room for an implementation of checked exceptions at a later time.)

Also from the same link:

The decision to use unchecked exceptions is a complicated one, and it's clear that there's no obvious answer. The Sun advice is to use them for nothing, the C# approach (which Eckel and others agree with) is to use them for everything. Others say, "there's a middle ground."

share|improve this answer
add comment

I would like to get comments on this, but I find there are times when this isn't necessarily bad practice. (Or terribly bad). But maybe i am wrong.

Often times an API you are using will throw an exception that you can't imagine actually being thrown in your specific usecase. In this case, it seems perfectly fine to throw a RuntimeException with the caught exception as the cause. If this exception is thrown, it'll likely be the cause of programming error and isn't inside the bounds for correct specification.

Assuming the RuntimeException isn't later caught and ignored, it's no where near an OnErrorResumeNext.

The OnErrorResumeNext would occur when someone catches an exception and simply ignores it or just prints it out. This is terribly bad practice in almost all cases.

share|improve this answer
    
This might be the case near the top of the call tree, where the only thing you can do is try to recover gracefully and knowing the specific error won't really help. In that case, you may have to record the error and move on (process next record, inform user that an error occurred, etc). Otherwise, no. You should always handle exceptions as close to the error as is practical, not wrap them up as a white elephant for the next handler. –  Michael K Nov 23 '11 at 19:47
add comment

It depends.

This practice may be even wise. There are many situations (for example in web developement), where if some exception happens, you are unable to do anything (because you cannot for example repair inconsistent DB from your code :-), only developer can do it). In these situations, it is wise to wrap the thrown exception into a runtime exception a rethrow it. Than you can catch all these exceptions in some exception handling layer, log the error and display the user some nice localized error code + message.

On the other hand, if the exception is not runtime (is checked), the developer of the API indicates, that this exception is resolvable and should be repaired. If its possible, than you should definitely do it.

The other solution might be to rethrow this checked exception into the calling layer, but if you were unable to solve it, where the exception occured, you will be likely unable to solve it here also...

share|improve this answer
    
You hope that the developer of the API knew what s/he was doing and used checked exceptions well. I started seeing APIs that favor throwing runtime exceptions while also documenting it so the client has the option to catch it if it wants to. –  c_maker Nov 23 '11 at 22:31
    
A method which throws an exception cannot generally know if a caller might recover from it. On the other hand, I would suggest that a method should only let a checked exception thrown by an inner method to escape if knows why the inner method threw the exception, and the reason is consistent with the outer method's API. If an inner method throws a checked exception unexpectedly, letting it bubble up as a checked exception may leave the caller misinformed as to what happened. –  supercat 17 hours ago
add comment

For standalone applications. When you know your application cannot handle the exception you could, instead of throwing the checked RuntimeException, throw Error, let the application crash, hope for bug-reports, and fix your application. (See the answer of mrmuggles for a more in depth discussion of the pro's and con's of checked versus unchecked.)

share|improve this answer
add comment

This might depend on case to case basis. In certain scenarios it is wise to do what your friend is doing, for example when you are exposing an api for some clients and you want the client to be least aware of the implementation details, where you know that certain implementation exceptions may be specific to implementation details and not exposable to the client.

By keeping the checked exceptions out of the way, you can expose api's that would enable the client to write cleaner code as the client itself might be pre-validating the exceptional conditions.

For example Integer.parseInt(String) takes a string and returns the integer equivalent of it and throws NumberFormatException in case the string is not numeric. Now imagine a form submission with a field age is converted through this method but the client would have already ensured validation on its part, so there's no point forcing the check of exception.

share|improve this answer
add comment

This is common practice in many frameworks. E.g. Hibernate does exactly this. The idea is that the APIs should not be intrusive for client side and Exception are intrusive since you must explicitly write code to handle them at that place where you call the api. But that place might not be the right place to handle the exception in the first place.
Actually to be honest this is a "hot" topic and many dispute so I will not take a side but I will say that what your friend does/proposes is not un-ordinary or uncommon.

share|improve this answer
add comment

In my opinion,

In the framework level, we should be catch runtime exceptions to reduce more block of try catch to the invoker in the same place.

In the application level, we rarely capture runtime exceptions and i think this practice was bad.

share|improve this answer
    
And what will you do with those exceptions at the framework level ? –  Matthieu Dec 13 '11 at 1:15
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.