Main method code entirely inside try/catch: Is it bad practice?
Solution 1:
Wrapping any piece of code in a try
/catch
block without a good reason is bad practice.
In the .NET programming model, exceptions should be reserved for truly exceptional cases or conditions. You should only try to catch exceptions that you can actually do something about. Furthermore, you should should hardly ever catch the base System.Exception
class (but rather prefer to catch the more specific, derived exception classes you can handle). And should a truly unexpected exception be encountered during the course of your program's execution, you actually should crash.
Obviously the "correct" answer would have to be made on a case-by-case basis, depending on what's going on inside that // code
placeholder in your catch
block. But if you're asking for a general rule or "best practice", you should always have a specific reason to catch exceptions, not just wrap all of your code in a giant try
/catch
block as a matter of course without thinking about it.
Note that if you're simply trying to catch any unhandled exceptions that might occur for the purposes of logging or error reporting, you should be using the AppDomain.UnhandledException
event. This is a notification-only event, so it doesn't allow you to handle those exceptions, but it is the right place to implement your logging or error reporting functionality after your application has crashed.
EDIT: As I was catching up on my reading of Raymond Chen's excellent blog, "The Old New Thing", I noticed that he had recently published an article on a similar topic. It's specific to COM, rather than the .NET Framework, but the general concepts regarding error handling are equally applicable to both environments. I thought I'd share a couple of gems from the article here, in support of my [apparently quite controversial] opinion.
Historically, COM placed a giant try/except around your server's methods. If your server encountered what would normally be an unhandled exception, the giant try/except would catch it and turn it into the error
RPC_E_SERVERFAULT
. It then marked the exception as handled, so that the server remained running, thereby "improving robustness by keeping the server running even when it encountered a problem."Mind you, this was actually a disservice.
The fact that an unhandled exception occurred means that the server was in an unexpected state. By catching the exception and saying, "Don't worry, it's all good," you end up leaving a corrupted server running.
[ . . . ]
Catching all exceptions and letting the process continue running assumes that a server can recover from an unexpected failure. But this is absurd. You already know that the server is unrecoverably toast: It crashed!
Much better is to let the server crash so that the crash dump can be captured at the point of the failure. Now you have a fighting chance of figuring out what's going on.
You can [and should] read the whole article here on his blog: How to turn off the exception handler that COM "helpfully" wraps around your server.
Solution 2:
If you're doing the most intelligent thing you can with the error, it's good practice. That's what try/catch is for.
If you're just throwing the error away (or logging it and throwing it away) — especially if you're doing so regardless of the type of exception — that's considered bad practice.
This might raise the question: what if the most intelligent thing is to log it and throw it away? I want to say that would be an exceptional case. But in practice, my code would assert otherwise. I guess I indulge in a lot of bad practice.