What's wrong with this code? (#3) - Answer
Here's the discussion for the my last post.
I liked Matthew's response:
...don't throw "Exception" since you are not supposed to catch "Exception." Anybody handling exceptions thrown by this code would have to catch Exception and ignore asynchronous exceptions (like OutOfMemoryException) at the same time, which is not easy.
To expand a bit, if you wrap in a class like Exception, you force the user to write something like:
try
{
account.UpdateBalance();
}
catch (Exception e)
{
if (e.InnerException.GetType() == typeof(DatabaseException))
{
// handle the exception here
}
else
{
throw;
}
}
This is really ugly, and if the user forgets the final "throw", the exception gets swallowed.
I also liked Steve's comment:
I don't like the fact that the exception message exposes both my account number and the account balance. If this exception text made it all the way back to the client, the current message seems like a privacy/security hole.
There are also likely things that need to be done to make the database part more robust, though that wasn't what I was intending to illustrate.
- Anonymous
November 21, 2004
You might want to change
if (e.InnerException.GetType() == typeof(DatabaseException))
to
if (typeof(DatabaseException).IsAssignableFrom(e.InnerException.GetType()))
This lets the called code throw a DatabaseException subclass in the future with more information, if they so wish, without breaking your code. Exact type comparison is rarely what people need in most situations... - Anonymous
November 21, 2004
Marcelo,
e.InnerException can be a null.
Much better is
(typeof(DatabaseException).IsInstanceOfType(e.InnerException)) - Anonymous
November 21, 2004
Or the best one
if (e.InnerException is DatabaseException) ;-) - Anonymous
November 25, 2004
Simplest is best...
if( e.InnerException is DatabaseException )
{ ... } - Anonymous
November 28, 2004
The comment has been removed - Anonymous
December 18, 2004
Helpful For MBA Fans. - Anonymous
December 27, 2004
[http://itpeixun.51.net/][http://aissl.51.net/][http://kukuxz003.freewebpage.org/][http://kukuxz001.51.net/][http://kukuxz003.51.net/][http://kukuxz005.51.net/][http://kukuxz002.51.net/][http://kukuxz004.freewebpage.org/][http://kukuxz007.51.net/][http://kukuxz001.freewebpage.org/][http://kukuxz006.51.net/][http://kukuxz002.freewebpage.org/][http://kukuxz004.51.net/][http://kukuxz008.51.net/][http://kukuxz009.51.net/][http://kukuxz005.freewebpage.org/][http://kukuxz006.freewebpage.org/][http://kukuxz007.freewebpage.org/][http://kukuxz009.freewebpage.org/]