Often when looking at others code reviews I am still surprised to see Exception catchalls. They only very rarely have a use. You should never throw “Exception()” or “SystemException()” or “ApplicationException()”. The reason for this is that Exception() is way to generic and cannot be handled well, and the other two are ideally reserved for the system to raise only. i.e.
void SomeMethod()
{
if (SomeReason)
throw new Exception("SomeReason occured");
if (SomeOtherReason)
throw new Exception("SomeOtherReason occured");
}
try
{
SomeMethod()
}
catch (Exception ex)
{
//handle
}
The only way you can handle the exception is looking at the message, which is really ugly, and if the message changes the exception trapping won’t work anymore. Not to mention the exception gets swallowed… You should never ever catch a Exception classes except at the outer most layer in which you would display the error in a user friendly way.
What you really want to do is create a specific exception class and then raise that exception. Then the code is cleaner, safer, more readable and other exceptions are propagated up to the client:
try
{
SomeMethod()
}
catch (ArgumentNullException ex)
{
//Shouldn't have passed a null exception in.
}