I never did any serious Java coding before, but I learned the syntax, libraries, and concepts based on my existing skills (Delphi & C#). One thing I hardly understand is that I've seen soo much code that silently consume exceptions after "printStackTrace" like this:
public void process() {
try {
System.out.println("test");
} catch(Exception e) {
e.printStackTrace();
}
}
There is similar code like this one in almost every Java article & project I ran into. Based on my knowledge this is very bad. The exception should almost always be forwarded to the outer context like this:
public void process() {
try {
System.out.println("test");
} catch(Exception e) {
e.printStackTrace();
throw new AssertionError(e);
}
}
Most of the time the exception should end up being handled at the outermost loop which belongs to the underlying framework (Java Swing for example). Why does it look like the norm to code like this in the Java world? I'm puzzled.
update :
May I add some more opinions. Based on my background, I'd prefer to remove printStackTrace AT ALL. I would simply rethrow as an unhandled aka RuntimeException. (or, even better, AssertionError), then catch and log it at the most appropriate place ... "the framework outermost loop".
public void process() {
try {
System.out.println("test");
} catch(Exception e) {
throw new AssertionError(e);
}
}
I have always thought, that's similar to the following scenario:
ReplyDelete"A man gets shot.
He holds his breath and has enough strength to take a bus.
10 miles later the man gets out of the bus, walks a couple of blocks and die."
When police get to the body, they don't have a clue of what just happened. They may have eventually but it is much harder.
Better is:
"One man gets shot, and the dies instantaneously and the body lays exactly where the murder had just happened"
When the police arrives, all the evidence is in place.
If a system is to fail, better is to fail fast
Addressing the question:
Ignorance.
+
Sloth
EDIT:
Of course, the catch section is useful.
If something can be done with the exception, that's where it should be done.
Probably that is NOT an exception for the given code, probably it is something that is expected ( and in my analogy is like a bulletproof jacket, and the man was waiting for the shot in first place ).
And yes, the catch could be used to Throw exceptions appropriate to the abstraction
Usually that is due to the IDE offering a helpful 'quick fix' that wraps the offending code in a try-catch block with that exception handling. The idea is that you actually DO something, but lazy developers don't.
ReplyDeleteThis is bad form, no doubt.
Java forces you to handle all Exceptions explicitly. If a method that your code calls is declared to throw FooException and BarException your code MUST handle (or throw) those exceptions. The only exception to this is RuntimeException, which is silent like a ninja.
ReplyDeleteLots of programmers are lazy (myself included), and it's very easy to just print the stack trace.
I find there are often 2 reasons this is done
ReplyDeleteProgrammer was lazy
Programmer wanted to guard an entry point into there component (correctly or incorrectly)
I do not believe this is a phenomenon limited to Java. I've seen such coding often in C# and VB.Net as well.
On the surface it's quite shocking and looks terrible. But really it's nothing new. It occurs all the time in C++ applications which use error code return values vs. exceptions. The difference though is that ignoring a potentially fatal return value doesn't really look any different than calling a function that returns void.
Foo* pFoo = ...;
pFoo->SomeMethod(); // Void or swallowing errors, who knows?
This code looks better but if SomeMethod() were to say return an HResult, it would be semantically no different than swallowing an exception.
This is a classic straw man argument. printStackTrace() is a debugging aid. If you saw it on a blog or in a magazine it was because the writer was more interested in illustrating a point other than exception handling. If you saw it in production code, the developer of that code was ignorant or lazy, nothing more. It shouldn't be held up as an example of common practice in the "java world".
ReplyDeletebecause Checked Exceptions is a failed experiment
ReplyDelete(maybe printStackTrace() is the real problem? :)
A System.out print or e.printStackTrace() - which implies use of System.out is usually a red flag meaning someone didn't bother to do a diligent job. Excepting desktop Java Applications, most Java apps are better off using logging.
ReplyDeleteIf the failure mode for a method is a no-operation, it's perfectly fine to eat an exception, whether you record the reason (and existence) or not. More typically, however, the catch clause should be taking some sort of exceptional action.
Rethrowing an exception is something that's best done when you either use the catch to clean up part of the work at a level where the necessary information is still available or when you need to transform the exception to an exception type more amenable to the caller.
I have to say I slightly resent the tone that implies this sort of lax error-handling behaviour is something fundamental to Java programmers. Sure, Java programmers can be lazy, just like every other programmer, and Java's a popular language, so you'll probably see a lot of code swallowing exceptions.
ReplyDeleteAlso, as has been pointed out elsewhere, there are understandable frustrations with Java's enforced declaration of checked exceptions, although personally I don't have a problem with that.
What I have a problem with, I guess, is that you're breezing through a bunch of articles and code snippets on the web without bothering to consider the context. The truth is, when you're writing a technical article trying to explain how some particular API works, or how to get started with something, then you're very likely to skip over some aspects of the code - the error handling that's not directly related to what you're demonstrating is a likely candidate for disposal, especially if the exception is unlikely to occur in the example scenario.
People who write articles of that nature have to maintain a reasonable signal-to-noise ratio, and rather fairly, I think, that means they have to assume you know some basics about the language you're developing in; how to deal properly with errors, and a bunch of other things. If you come across an article and notice a lack of proper error checking, then that's fine; just make sure that when you incorporate those ideas (but of course, never the exact code itself, right?) into your production code, you'll deal with all those bits and bobs that the author sensibly left out, in a manner that's most suited to what you're developing.
I do have a problem with very high-level introductory articles that breeze over such issues without ever returning to them, but please be aware that there's not some particular "mindset" of Java programmers regarding error handling; I know of plenty of your beloved C# programmers who don't bother dealing with all their problems, either.
It is only consumed silently if the catch block is empty really.
ReplyDeleteAs far as articles goes they are probably more interesting in proving some other point besides how to deal with exceptions. They just want to get straight to the point and have the shortest possible code.
Obviously you are right though, exceptions should at least be logged if they are going to be 'ignored'.
As others have pointed out, the reason you see this is for one of three reasons:
ReplyDeleteAn IDE generated the try-catch block
The code was copied and pasted
The developer put the stacktrace in to debug but never came back to handle the exception properly
The last point is the least likely to occur. I say this because I don't think anyone really debugs this way. Stepping through code with a debugger is a much easier way to debug.
The best description of what should be done in a catch block can be found in Chapter 9 of Effective Java by Joshua Bloch.
In C# all exceptions are runtime exceptions, but in Java you have runtime exceptions and checked exceptions, that you have to either catch, or declare in your methods. If you call any method that has a "throws" at the end, you have to either catch the exceptions mentioned there, or your method has to also declare those exceptions.
ReplyDeleteJava Articles usually just print the stack trace or have a comment because the exception handling is irrelevant to the article's subject matter. In projects though, something should be done about it, depending on the type of exception.
You should see this very often if the programmer does his job right. Ignoring Exception is a bad, bad practice! But there are some reasons why some might do this and the more apporiate solutions:
ReplyDelete"This wont happen!"
Sure, sometime you "know" that this Exception won't happen, but its still more appropiate to rethrow a runtime exception whith the occoured Exception as "cause" instead of just ignoring it. I bet it will occour sometime in the future. ;-)
Prototyping Code
If you're just typing your stuff down to see if it works out you might want to ignore all Exceptions that might occour. This is the only case i do some lazy catch(Throwable). But if the code will turn out into something useful, i include proper exception handling.
"I dont know what to do!"
I saw much code, especially library code, which swallows occouring exceptions because in this layer of the application no proper handling can be done. DONT DO THIS! Just rethrow the exception (either by adding a throws clause in the method signature or wrapping the exception into a library specific one).
You should always forward it on or handle it suitably in a real-world context. A lot of articles and tutorials will simplify their code to get the more important points across though and one of the easiest things to simplify is error-handling (unless what you are doing is writing an article on error-handling that is :)). As java code will check for exception handling then putting a simple silent (or logging statement) catch block on is the simplest method to provide a working example.
ReplyDeleteIf you find this in anything other than example code, feel free to forward the code onto TDWTF, although they may have too many examples of it by now :)
The combination of checked exceptions and interfaces lead to the situation that the code must handle exections that never ever get thrown. (The same applies to normal inheritance, too, but it's more common and easier to explain with interfaces)
ReplyDeleteReason: The implementation of an interface may not throw (checked) exceptions other than those defined in the interface specification. For that reason, the creators of an interface, not knowing which methods of a class implementing the interface might actually need to throw an exception, might specifiy that all methods might throw at least one type of exception. Example: JDBC, where everything and its grandma is declared to throw SQLException.
But in reality, many methods of real implementations simply cannot fail, so under no circumstances, they ever throw an exception. Code calling this methods must still somehow "handle" the exception, and the easiest way is to do that swallow the exception. Nobody wants to clutter his code with seemingly useless error-handling that never ever gets executed.
The real point of exceptions is to simplify error handling and separate it from error detection. This is in contrary to representing errors by error codes, where error
ReplyDeletehandling code is scattered everywhere and every call which may fail shall be checked
for return code.
If exception represents an error (which is most of the cases) usually the most reasonable way to handle it is to bail out and leave the handling to some upper layer.
Rethrowing different exception should be considered if some meaningful semantics is added to it i.e., this error is an unusual system failure / temporary (networking) problem / this is client or server side error etc.
Of all error handling strategies the most ignorant is hiding or simply printing error massage and going forward as nothing happened.
Sun folks wanted the code to be more explicit and forced programmers to write which exceptions may be thrown by which method. It seemed to be right move -- anybody will
known what to expect in return from any method call given it's prototype (it may return
value of this type or throw an instance of one of the specified classes (or it's subclass)).
But as it turned out with lots of ignorant Java programmers they now treat exception handling as if it was a language bug/"feature" which needed a workaround and write code in worst or almost worst possible way:
The error is handled right away in context not suitable to decide what to do with it.
It is displayed or ignored silently and computing continues even when further code has
no chance to run properly.
The caller of method can not differentiate whether it finished successfully or not.
How to write the "right way" than?
Indicate every base class of exceptions which can be thrown in method header.
AFAICR Eclipse can do it automatically.
Make the throw list in method prototype meaningful.
Long lists are pointless and "throw Exception" is lazy (but useful when you not bother
much about exceptions).
When writing the "wrong way" simple "throw Exception" is much better and takes
less bytes than "try{ ... } catch(Exception e) { e.printStackTrace(); }".
Rethrow chained exception if needed.
Because they haven't learned this trick yet:
ReplyDeleteclass ExceptionUtils {
public static RuntimeException cloak(Throwable t) {
return ExceptionUtils.<RuntimeException>castAndRethrow(t);
}
@SuppressWarnings("unchecked")
private static <X extends Throwable> X castAndRethrow(Throwable t) throws X {
throw (X) t;
}
}
class Main {
public static void main(String[] args) { // Note no "throws" declaration
try {
// Do stuff that can throw IOException
} catch (IOException ex) {
// Pretend to throw RuntimeException, but really rethrowing the IOException
throw ExceptionUtils.cloak(ex);
}
}
}
As pointed out, calling printStackTrace() isn't really silent handling.
ReplyDeleteThe reason for this sort of "swallowing" of the exception is that, if you keep passing the exception up the chain, you still have to handle the exception somewhere or let the application crash. So, handling it at the level it occurs with an information dump is no worse than handling it at the top level with an information dump.
If you want your exception to be handled outside the scope of the current method you don't need to to catch it actually, instead you add 'throws' statement to the method signature.
ReplyDeleteThe try/catch statements that you've seen are only in the code where programmer explicitly decided to handle the exception in place and therefore not to throw it further.
I think developers also try to consider the importance of "doing the right thing" in a particular context. Often times for throw away code or when propagating the exception upwards wouldn't buy anything because the exception is fatal, you might as well save time and effort by "doing the wrong thing".
ReplyDeletebecause it is a best practice. I thought everybody knew.
ReplyDeletebut the cold truth is that nobody really understands how to work with exceptions. The C error handing style made so much more sense.
Its lazy practice - nothing short of it really.
ReplyDeleteIts usually done when you really don't care about the exception - rather than increasing your finger-work.
You would usually swallow an exception when you cannot recover from it but it is not critical. One good example is the IOExcetion that can be thrown when closing a database connection. Do you really want to crash if this happens? That's why Jakarta's DBUtils have closeSilently methods.
ReplyDeleteNow for checked exception that you cannot recover but are critical (usually due to programming errors), don't swallow them. I think exceptions should be logged the nearest as possible to the source of the problem so I would not recommend removing the printStackTrace() call. You will want to turn them into RuntimeException for the sole purpose of not having to declare these exceptions in you business method. Really it doesn't make sense to have high level business methods such as createClientAccount() throws ProgrammingErrorException (read SQLException), there is nothing you can do if you have typos in your sql or accessing bad indexes.
I disagree that rethrowing a checked exception is a better idea. Catching means handling; if you have to rethrow, you shouldn't catch. I'd add the throws clause to the method signature in that case.
ReplyDeleteI would say that wrapping a checked exception in an unchecked one (e.g., the way Spring wraps the checked SQLException into an instance of its unchecked hierarchy) is acceptable.
Logging can be considered handling. If the example was changed to log the stack trace using log4j instead of writing to the console, would that make it acceptable? Not much of a change, IMO.
The real issue is what is considered exceptional and an acceptable recovery procedure. If you can't recover from the exception, the best you can do is report the failure.
From experience, Swallowing an exception is harmful mostly when it's not printed. It helps to bring attention if you crash, and I'll do that deliberately at times, but simply printing the exception and continuing allows you to find the problem and fix it, and yet usually doesn't negatively effect others working on the same codebase.
ReplyDeleteI'm actually into Fail-Fast/Fail-HARD, but at least print it out. If you actually "Eat" an exception (which is to truly do nothing: {}) It will cost someone DAYS to find it.
The problem is that Java forces you to catch a lot of stuff that the developer knows won't be thrown, or doesn't care if they are. The most common is Thread.sleep(). I realize there are holes that might allow threading issues here, but generally you know that you are not interrupting it. Period.
I'm afraid that most of java programmers do not know what to do with Exceptions, and quite always consider it as an annoyance that slows their coding of the "nominal" case.
ReplyDeleteOf course they're totally wrong, but it's difficult to convince them that IT IS important to correctly deal with exceptions.
Every time I encounter such a programmer (it happens frequently) I give him two reading entries :
the famous Thinking In java
A short and interesting article of Barry Ruzek available here : www.oracle.com/technology/pub/articles/dev2arch/2006/11/effective-exceptions.html
By the way, I strongly agree that it is stupid to catch a typed exception to rethrow it embedded in a RuntimeException :
if you catch it, HANDLE it.
otherwise, change your method signature to add possible exceptions that you would / could not handle, so your caller would have a chance to do it on his own.
There can be many reasons why one would use catch Exception. In many cases it is a bad idea because you also catch RuntimeExceptions - and well you don't know in what state the underlying objects will be after this happens ? That is always the difficult thing with unexpected conditions: can you trust that the rest of the code will not fail afterwards.
ReplyDeleteYour example prints the stacktrace so at least your will know what the root cause might have been. In bigger software projects it is a better idea to log these things. And lets hope that the log component does not throw exceptions either our you might end up in an infinite loop (which will probably kill your JVM).
If you have a checked exception and you don't want to handle it in a method, you should just have the method throw the exception. Only catch and handle exceptions if you are going to do something useful with it. Just logging it is not very useful in my book as users rarely have time to be reading logs looking for exceptions or know what to do if an exception is thrown.
ReplyDeleteWhile wrapping the exception is an option, I would not suggest you do this unless; you are throwing a different exception to match an exist interface or there really is no way such an exception should be thrown.
BTW: If you want to re throw a checked exception you can do this with
try {
// do something
} catch (Throwable e) {
// do something with the exception
Thread.currentThread().stop(e); // doesn't actually stop the current thread, but throws the exception/error/throwable
}
Note: if you do this, you should make sure the throws declaration for the method is correct as the compiler is unable to do this for you in this situation.
Please don't ever, ever, ever wrap a checked exception in an unchecked exception.
ReplyDeleteIf you find yourself dealing with exceptions that you don't think you should then my advice is that you are probably working at the wrong level of abstraction.
I'll elaborate: checked and unchecked exceptions are two very different beasts. Checked exceptions are similar to the old method of returning error codes... yes, they are somewhat more painful to handle than error codes but they have advantages too. Unchecked exceptions are programming errors and critical system failures... exceptional exceptions in other words. When trying to explain what an exception is a lot of people get into a complete mess because they don't acknowledge the difference in these two, very different, cases.
How do you know people frequently consume exceptions? Has this been studied? Where is this study published?
ReplyDelete