Exceptions as return values

[dcassar]

I have had a lively discussion with some coworkers and decided to get
some general feedback on an issue that I could find very little
guidance on. Why is it considered bad practice to define a public
member with a return type that is derived from System.Exception? I
understand the importance of having clean, concise code that follows
widely-accepted patterns and practices, but in this case, I find it
hard to blindly follow a standard when a better, more elegant solution
presents itself so readily.

I was writing a generic function to validate data that is going to be
used to represent key values for my business objects. It looked like
this:

public static ValidationException ValidateKeyArray(System.Type
targetType, object[] keys);

ValidationException is derived from ApplicationException, which, in
turn, derives from the base Exception class. The reason I chose to
write this in this way was to meet three requirements for this method:
a) Validation in itself should not throw an exception unless it cannot
figure out how to validate for some reason.
b) The code calling this method should be able to determine why
validation failed so it can either respond appropriately or throw an
exception if it can't recover
c) Figuring out why validation failed is not a separate logical process
from performing the validation itself, so it makes sense to have a
single function call to do both.

With the function written in this way (returning an object of type
ValidationException) I can:
1) Throw an exception if validation is impossible due to bad input, but
not have to throw an exception if the data is in the correct format but
is invalid
2) Return a descriptive error message in the ValidationException object
3) Return multiple error messages, if necessary (embedded in the
ValidationException object or attached to inner exceptions)
4) Avoid breaking normal program flow when validating, since failed
validation is not an error condition
5) Provide a throwable object to the caller so that critical validation
can be performed in two lines (ValidationException x =
ValidateKeyArray(type, array); if (x != null) throw x

As I understand them, the major alternatives are:

1) Returning a bool. This does not meet requirements (b) and (c).

2) Exposing validation information on the object, like the ASP.NET Page
object does. This would require a Validate method and an IsValid
property, and perhaps some sort of list to provide validation error
messages. This is considerably more code than the one I implemented,
and does not meet requirement (c). Also, validation exceptions would
not be standardized in any way, since I would be relying on the calling
code to throw an exception if it needs to.

3) Always throwing exceptions from the ValidateKeyArray method, and
relying on the caller to use a try...catch block to manage it. If you
know that the input could be bad, shouldn't you avoid throwing an
exception at all?

4) Returning some sort of non-Exception-derived class to expose the
results of validation. The only real disadvantage here is that it
requires some extra code to extract the message from this object and
then put it into an Exception if you need to throw it.

I am looking forward to hearing your thoughts.

Daniel

 

[Craig S]

As I read your post, I went through a series of thoughts on the topic.
Initially, I thought you were nuts and you should either bubble up the
exception or catch it and handle it. However, it seems like you put a lot
of thought into the reasoning behind it and it's possible I could be
convinced if I read it again - not sure. I think the biggest problem is
just that the initial reaction is that it's counterintuitive to some.

Is there a better way to do it? I'm going to let someone else tackle that,
it would take a lot of thought and I'm on a pretty tight deadline for the
next couple weeks.

Craig

I have had a lively discussion with some coworkers and decided to get
some general feedback on an issue that I could find very little
guidance on. Why is it considered bad practice to define a public
member with a return type that is derived from System.Exception? I
understand the importance of having clean, concise code that follows
widely-accepted patterns and practices, but in this case, I find it
hard to blindly follow a standard when a better, more elegant solution
presents itself so readily.

I was writing a generic function to validate data that is going to be
used to represent key values for my business objects. It looked like
this:

public static ValidationException ValidateKeyArray(System.Type
targetType, object[] keys);

ValidationException is derived from ApplicationException, which, in
turn, derives from the base Exception class. The reason I chose to
write this in this way was to meet three requirements for this method:
a) Validation in itself should not throw an exception unless it cannot
figure out how to validate for some reason.
b) The code calling this method should be able to determine why
validation failed so it can either respond appropriately or throw an
exception if it can't recover
c) Figuring out why validation failed is not a separate logical process
from performing the validation itself, so it makes sense to have a
single function call to do both.

With the function written in this way (returning an object of type
ValidationException) I can:
1) Throw an exception if validation is impossible due to bad input, but
not have to throw an exception if the data is in the correct format but
is invalid
2) Return a descriptive error message in the ValidationException object
3) Return multiple error messages, if necessary (embedded in the
ValidationException object or attached to inner exceptions)
4) Avoid breaking normal program flow when validating, since failed
validation is not an error condition
5) Provide a throwable object to the caller so that critical validation
can be performed in two lines (ValidationException x =
ValidateKeyArray(type, array); if (x != null) throw x

As I understand them, the major alternatives are:

1) Returning a bool. This does not meet requirements (b) and (c).

2) Exposing validation information on the object, like the ASP.NET Page
object does. This would require a Validate method and an IsValid
property, and perhaps some sort of list to provide validation error
messages. This is considerably more code than the one I implemented,
and does not meet requirement (c). Also, validation exceptions would
not be standardized in any way, since I would be relying on the calling
code to throw an exception if it needs to.

3) Always throwing exceptions from the ValidateKeyArray method, and
relying on the caller to use a try...catch block to manage it. If you
know that the input could be bad, shouldn't you avoid throwing an
exception at all?

4) Returning some sort of non-Exception-derived class to expose the
results of validation. The only real disadvantage here is that it
requires some extra code to extract the message from this object and
then put it into an Exception if you need to throw it.

I am looking forward to hearing your thoughts.

Daniel

 

[Guest]

Daniel,

I think the main issue here is one of logical constructs. A validation
method would normally return true or false - that's accepted practice. If
you feel your validation method needs to return more information -- such as
why validation failed, etc., I'd consider creating a ValidationResult class
containing this additional information, and return an instance of that rather
than an exception.
Exceptions are expensive, rack up a lot of CPU cycles, and they're really
for one purpose - something Exceptional.

Validation failing is a normal and expected business logic result, and
should not result in an exception.
Hope that helps,
Peter


--
Co-founder, Eggheadcafe.com developer portal:
http://www.eggheadcafe.com
UnBlog:
http://petesbloggerama.blogspot.com

I have had a lively discussion with some coworkers and decided to get
some general feedback on an issue that I could find very little
guidance on. Why is it considered bad practice to define a public
member with a return type that is derived from System.Exception? I
understand the importance of having clean, concise code that follows
widely-accepted patterns and practices, but in this case, I find it
hard to blindly follow a standard when a better, more elegant solution
presents itself so readily.

I was writing a generic function to validate data that is going to be
used to represent key values for my business objects. It looked like
this:

public static ValidationException ValidateKeyArray(System.Type
targetType, object[] keys);

ValidationException is derived from ApplicationException, which, in
turn, derives from the base Exception class. The reason I chose to
write this in this way was to meet three requirements for this method:
a) Validation in itself should not throw an exception unless it cannot
figure out how to validate for some reason.
b) The code calling this method should be able to determine why
validation failed so it can either respond appropriately or throw an
exception if it can't recover
c) Figuring out why validation failed is not a separate logical process
from performing the validation itself, so it makes sense to have a
single function call to do both.

With the function written in this way (returning an object of type
ValidationException) I can:
1) Throw an exception if validation is impossible due to bad input, but
not have to throw an exception if the data is in the correct format but
is invalid
2) Return a descriptive error message in the ValidationException object
3) Return multiple error messages, if necessary (embedded in the
ValidationException object or attached to inner exceptions)
4) Avoid breaking normal program flow when validating, since failed
validation is not an error condition
5) Provide a throwable object to the caller so that critical validation
can be performed in two lines (ValidationException x =
ValidateKeyArray(type, array); if (x != null) throw x

As I understand them, the major alternatives are:

1) Returning a bool. This does not meet requirements (b) and (c).

2) Exposing validation information on the object, like the ASP.NET Page
object does. This would require a Validate method and an IsValid
property, and perhaps some sort of list to provide validation error
messages. This is considerably more code than the one I implemented,
and does not meet requirement (c). Also, validation exceptions would
not be standardized in any way, since I would be relying on the calling
code to throw an exception if it needs to.

3) Always throwing exceptions from the ValidateKeyArray method, and
relying on the caller to use a try...catch block to manage it. If you
know that the input could be bad, shouldn't you avoid throwing an
exception at all?

4) Returning some sort of non-Exception-derived class to expose the
results of validation. The only real disadvantage here is that it
requires some extra code to extract the message from this object and
then put it into an Exception if you need to throw it.

I am looking forward to hearing your thoughts.

Daniel

 

[Nicholas Paldino [.NET/C# MVP]]

Daniel,

I have mixed feelings about this. Also, I think that there needs to be
a correction.

Peter wasn't too clear when he said that exceptions are expensive.
Exceptions are expensive if they are thrown. Creating them, on the other
hand, isn't that expensive. When an exception is created, the stack trace
isn't populated (which is the one thing I can think of which would be
expensive in constructing an exception, but the stack trace is populated
when it is thrown, not when it is created, so it is a non issue).

Now, there is a lot of ingrained thinking about using exceptions like
other objects. We are pretty much given the impression that they are only
to be thrown.

However, they seem to suit your need, and because you are reusing
existing code to suit your needs, it works out well.

I agree with these reasons for using it, and on the surface, it is a
good idea.

However, there is one thing that has been neglected in this discussion
up to this point. That is the fact that whatever you return can be thrown
by any section of code. This is a BIG issue, IMO, as you are giving other
code to really throw something which is specific to your problem domain.
That's just not cool. Granted, in C++, you can throw anything (it doesn't
have to be limited to an object derived from exception), but in most other
..NET languages, you have to derive from exception.

For that reason alone, I think that it is a good idea to create your own
class which will expose the properties you need, which indicates the error
states/information for failed buisiness logic (but not critical errors).

Hope this helps.


--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)

I have had a lively discussion with some coworkers and decided to get
some general feedback on an issue that I could find very little
guidance on. Why is it considered bad practice to define a public
member with a return type that is derived from System.Exception? I
understand the importance of having clean, concise code that follows
widely-accepted patterns and practices, but in this case, I find it
hard to blindly follow a standard when a better, more elegant solution
presents itself so readily.

I was writing a generic function to validate data that is going to be
used to represent key values for my business objects. It looked like
this:

public static ValidationException ValidateKeyArray(System.Type
targetType, object[] keys);

ValidationException is derived from ApplicationException, which, in
turn, derives from the base Exception class. The reason I chose to
write this in this way was to meet three requirements for this method:
a) Validation in itself should not throw an exception unless it cannot
figure out how to validate for some reason.
b) The code calling this method should be able to determine why
validation failed so it can either respond appropriately or throw an
exception if it can't recover
c) Figuring out why validation failed is not a separate logical process
from performing the validation itself, so it makes sense to have a
single function call to do both.

With the function written in this way (returning an object of type
ValidationException) I can:
1) Throw an exception if validation is impossible due to bad input, but
not have to throw an exception if the data is in the correct format but
is invalid
2) Return a descriptive error message in the ValidationException object
3) Return multiple error messages, if necessary (embedded in the
ValidationException object or attached to inner exceptions)
4) Avoid breaking normal program flow when validating, since failed
validation is not an error condition
5) Provide a throwable object to the caller so that critical validation
can be performed in two lines (ValidationException x =
ValidateKeyArray(type, array); if (x != null) throw x

As I understand them, the major alternatives are:

1) Returning a bool. This does not meet requirements (b) and (c).

2) Exposing validation information on the object, like the ASP.NET Page
object does. This would require a Validate method and an IsValid
property, and perhaps some sort of list to provide validation error
messages. This is considerably more code than the one I implemented,
and does not meet requirement (c). Also, validation exceptions would
not be standardized in any way, since I would be relying on the calling
code to throw an exception if it needs to.

3) Always throwing exceptions from the ValidateKeyArray method, and
relying on the caller to use a try...catch block to manage it. If you
know that the input could be bad, shouldn't you avoid throwing an
exception at all?

4) Returning some sort of non-Exception-derived class to expose the
results of validation. The only real disadvantage here is that it
requires some extra code to extract the message from this object and
then put it into an Exception if you need to throw it.

I am looking forward to hearing your thoughts.

Daniel

 

[Brant Estes]

One thing you need to realize, that if you're validating information
(such as user input), then effectively, you're making sure that there
isn't bad data. Exceptions are for exceptional circumstances. Bad
data isn't an exceptional circumstance, it happens all the time!

There tends to be great deal of difference between definitions of an
"Exception". Exceptions are extremely expensive to throw and catch,
therefore, if you can avoid them or come up with alternatives, then by
all means, do it! To prove to a coworker on how expensive it is to
throw and catch exceptions, I set up a simple example. Take the
DivideByZeroException for example. This is thrown whenever you attempt
to divide an integer by zero. The reason this is thrown, is that there
is absolutely no way whatsoever that that particular operation can
continue, because of the bounds of mathematics. So, take the following
samples on how to address this problem.

public int DivideBy(int divisor)
{
try
{
return 1 / divisor;
}
catch (Exception ex)
{
return 0;
}
}

This simply will attempt to divide 1 by the number entered into the
method. If it fails, it just returns 0. Now, take this example:

public int DivideBy(int divisor)
{
if (divisor != 0)
return 1 / divisor;
else
return 0;
}

This is slightly different, but with the same results. Identical? Not
even close! In my test to a coworker, I called each method one million
times, to see how long it would take. The method which checks for zero
first took only a few milliseconds to race through the method a million
times. The version which used brute force, and wrapped it in a
try/catch block took over 5 minutes!!

Here are a few guidelines on when and when not to use exceptions.

1) When a method absolutely, positively cannot complete what it is
meant to do, and has exhausted every last possible work around, and
simply cannot recover from something, then an exception should be
thrown. Examples of this would be necessary resources are not present,
such as a database, network connection, web service, file system,
security credentials, etc. Example: The File.Open() method cannot open
a file because it does not exist. Its purpose is to open a file, and
stream the contents back to you. If the file does not exist, it cannot
perform its intended action. It cannot complete its purpose in life.
It doesn't create a new file, because that's not its purpose. It
doesn't send another file, because that's not its purpose either.
However, there IS a work around! File.Exists() is provided to avoid
that exception altogether! There IS a work around without the need for
a try/catch block around the File.Open() method.

2) Do NOT use an exception to communicate between classes/layers/tiers.
Unless you're unit testing, there is no such thing as an "Expected
Exception". Business logic should not contain reasoning saying, "This
method _might_ throw an exception 50% of the time for various reasons,
so its important to have it inside of a try/catch block." This goes
back to the expensive nature of exceptions. Returning an exception as
you propose is interesting, but doesn't hold true to the concept of
validation. Validating that a correct username and password is passed
by the user has a very high likelyhood that they would come back wrong.
An exception definitely shouldn't be "bubbled up" from any of the
tiers for an event like this. Its expected behavior, and should be
handled through events, method return values, or some other mechanism.
However, "The database server is currently on fire, and the network
cables have melted" is certainly an exception. You NEVER anticipated
this to ever happen, and honestly, short of automatically calling the
fire department, this is not recoverable.

Proper coding techniques can greatly reduce the amount of exceptions
that we have to deal with on a day-to-day basis. I'm sure we've all
gotten NullReferenceExceptions at least 50 times a week:

string a = myObject.ToString(); // throws an exception is myObject is
null.

The answer isn't to wrap it with a try/catch block:

try
{
string a = myObject.ToString();
}
catch {}

This is just expensive, especially since there is a much better work
around:

if (myObject != null)
string a = myObject.ToString();

I'm in no way, shape, or form advocating against exceptions, or
try/catch blocks, however, I've seen them abused, misused, and overused
almost daily. They have their purpose in modern day programming, and
their proper use will make us all better programmers.

Hope this helps!

Brant Estes
Senior Consultant
Magenic Technologies

 

[David Levine]

I agree with all your points - especially about ingrained attitudes.
Anytime thinking is replaced with rote behavior it leads to unfortunate
results.

I'd add one thing to your points. The OP stated he derived his exception
from the ApplicationException class; I feel this is a rather useless
exception type; it does nothing to communicate the cause of an exception,
and it adds depth to the exception hierarchy but adds little of value. I
prefer to derive directly from Exception, or perhaps use a more descriptive
exception type, such as ArgumentException, rather then ApplicationException
or a custom exception. I prefer to avoid custom exception types unless I
have a great deal of control over their usage - if the exception were to be
propagated across a appdomain, process, or machine boundary, then unless the
assembly that contains the definition of the custom exception is present and
of the correct version, the exception object cannot be deserialized on the
client side of the boundary, resulting in a different, and very likely,
misleading exception. To avoid this I prefer to use standard exception types
defined in the BCL.

I like exposing the exception as a property of the validation class; as the
OP stated, this provides the user of the class the option of how to deal
with the exception. It also provides the option for the client to throw the
exception on a different thread if necessary. For example, the client can
launch a worker thread to process a number items to be validated, and any
exceptions can be rethrown on the original thread when the worker thread is
done.

regards,
Dave

Daniel,

I have mixed feelings about this. Also, I think that there needs to be
a correction.

Peter wasn't too clear when he said that exceptions are expensive.
Exceptions are expensive if they are thrown. Creating them, on the other
hand, isn't that expensive. When an exception is created, the stack trace
isn't populated (which is the one thing I can think of which would be
expensive in constructing an exception, but the stack trace is populated
when it is thrown, not when it is created, so it is a non issue).

Now, there is a lot of ingrained thinking about using exceptions like
other objects. We are pretty much given the impression that they are only
to be thrown.

However, they seem to suit your need, and because you are reusing
existing code to suit your needs, it works out well.

I agree with these reasons for using it, and on the surface, it is a
good idea.

However, there is one thing that has been neglected in this discussion
up to this point. That is the fact that whatever you return can be thrown
by any section of code. This is a BIG issue, IMO, as you are giving other
code to really throw something which is specific to your problem domain.
That's just not cool. Granted, in C++, you can throw anything (it doesn't
have to be limited to an object derived from exception), but in most other
.NET languages, you have to derive from exception.

For that reason alone, I think that it is a good idea to create your
own class which will expose the properties you need, which indicates the
error states/information for failed buisiness logic (but not critical
errors).

Hope this helps.


--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)

I have had a lively discussion with some coworkers and decided to get
some general feedback on an issue that I could find very little
guidance on. Why is it considered bad practice to define a public
member with a return type that is derived from System.Exception? I
understand the importance of having clean, concise code that follows
widely-accepted patterns and practices, but in this case, I find it
hard to blindly follow a standard when a better, more elegant solution
presents itself so readily.

I was writing a generic function to validate data that is going to be
used to represent key values for my business objects. It looked like
this:

public static ValidationException ValidateKeyArray(System.Type
targetType, object[] keys);

ValidationException is derived from ApplicationException, which, in
turn, derives from the base Exception class. The reason I chose to
write this in this way was to meet three requirements for this method:
a) Validation in itself should not throw an exception unless it cannot
figure out how to validate for some reason.
b) The code calling this method should be able to determine why
validation failed so it can either respond appropriately or throw an
exception if it can't recover
c) Figuring out why validation failed is not a separate logical process
from performing the validation itself, so it makes sense to have a
single function call to do both.

With the function written in this way (returning an object of type
ValidationException) I can:
1) Throw an exception if validation is impossible due to bad input, but
not have to throw an exception if the data is in the correct format but
is invalid
2) Return a descriptive error message in the ValidationException object
3) Return multiple error messages, if necessary (embedded in the
ValidationException object or attached to inner exceptions)
4) Avoid breaking normal program flow when validating, since failed
validation is not an error condition
5) Provide a throwable object to the caller so that critical validation
can be performed in two lines (ValidationException x =
ValidateKeyArray(type, array); if (x != null) throw x

As I understand them, the major alternatives are:

1) Returning a bool. This does not meet requirements (b) and (c).

2) Exposing validation information on the object, like the ASP.NET Page
object does. This would require a Validate method and an IsValid
property, and perhaps some sort of list to provide validation error
messages. This is considerably more code than the one I implemented,
and does not meet requirement (c). Also, validation exceptions would
not be standardized in any way, since I would be relying on the calling
code to throw an exception if it needs to.

3) Always throwing exceptions from the ValidateKeyArray method, and
relying on the caller to use a try...catch block to manage it. If you
know that the input could be bad, shouldn't you avoid throwing an
exception at all?

4) Returning some sort of non-Exception-derived class to expose the
results of validation. The only real disadvantage here is that it
requires some extra code to extract the message from this object and
then put it into an Exception if you need to throw it.

I am looking forward to hearing your thoughts.

Daniel

 

[Jon Skeet [C# MVP]]

One thing you need to realize, that if you're validating information
(such as user input), then effectively, you're making sure that there
isn't bad data. Exceptions are for exceptional circumstances. Bad
data isn't an exceptional circumstance, it happens all the time!

Not "all the time" I'd say. Reasonably frequently - but exceptions
aren't actually *that* expensive IMO. I think it's very rare that
choosing to use an exception when it's not *obviously* wrong would
actually add a significant overhead.

Unless you're *really* throwing exceptions in a pretty tight loop (like
in your example) they're unlikely to make much difference. Usually
exceptions bubble up several stack frames, which usually means they're
*not* going to be part of a tight loop (and shouldn't be).

Note that if you're validating user input, the time taken to throw an
exception is going to be *much* shorter than the time taken to accept
the input in the first place...

There tends to be great deal of difference between definitions of an
"Exception". Exceptions are extremely expensive to throw and catch,
therefore, if you can avoid them or come up with alternatives, then by
all means, do it! To prove to a coworker on how expensive it is to
throw and catch exceptions, I set up a simple example. Take the
DivideByZeroException for example. This is thrown whenever you attempt
to divide an integer by zero. The reason this is thrown, is that there
is absolutely no way whatsoever that that particular operation can
continue, because of the bounds of mathematics. So, take the following
samples on how to address this problem.

public int DivideBy(int divisor)
{
try
{
return 1 / divisor;
}
catch (Exception ex)
{
return 0;
}
}

This simply will attempt to divide 1 by the number entered into the
method. If it fails, it just returns 0. Now, take this example:

public int DivideBy(int divisor)
{
if (divisor != 0)
return 1 / divisor;
else
return 0;
}

This is slightly different, but with the same results. Identical? Not
even close! In my test to a coworker, I called each method one million
times, to see how long it would take. The method which checks for zero
first took only a few milliseconds to race through the method a million
times. The version which used brute force, and wrapped it in a
try/catch block took over 5 minutes!!

In a debugger, I suspect. What do you get if you run the following from
a command line? On my box, it took 15 seconds - I doubt that my laptop
is 20 times faster than your box! Throwing exceptions is *much* slower
in a debugger than not, which is something people often overlook.

using System;

class Test
{
static void Main()
{
DateTime start = DateTime.Now;
for (int i=0; i < 1000000; i++)
{
DivideBy(0);
}
Console.WriteLine (DateTime.Now-start);
}

public static int DivideBy(int divisor)
{
try
{
return 1 / divisor;
}
catch (Exception)
{
return 0;
}
}

}

2) Do NOT use an exception to communicate between classes/layers/tiers.
Unless you're unit testing, there is no such thing as an "Expected
Exception". Business logic should not contain reasoning saying, "This
method _might_ throw an exception 50% of the time for various reasons,
so its important to have it inside of a try/catch block." This goes
back to the expensive nature of exceptions.

It shouldn't. The use of exceptions should be much more due to what
ends up being the most elegant code rather than performance unless the
exceptions are *shown* to cause a performance problem. Why bend a
design out of shape for the sake of something which may well not be a
significant overhead anyway?

I'm in no way, shape, or form advocating against exceptions, or
try/catch blocks, however, I've seen them abused, misused, and overused
almost daily. They have their purpose in modern day programming, and
their proper use will make us all better programmers.

Likewise I'm not advocating the use of exceptions everywhere. However,
I've seen return codes abused, misused and overused frequently, often
because people have an exaggerated view of how expensive exceptions
are. (The DivideByZeroException thrown in your example is relatively
expensive, actually - if you change the code to just throw and catch an
exception, it takes half as long on my box.)

See http://www.pobox.com/~skeet/csharp/exceptions.html for more on my
views on this.

 

[Jon Skeet [C# MVP]]

Jon wrote:

In a debugger, I suspect. What do you get if you run the following from
a command line? On my box, it took 15 seconds - I doubt that my laptop
is 20 times faster than your box! Throwing exceptions is *much* slower
in a debugger than not, which is something people often overlook.

I've just tried this again with .NET 2.0, and it's more than twice as
slow, both at explicitly throwing and catching an exception and in the
division by zero situation. Maybe that's accounting for some of the
discrepancy?

Either way, I'd certainly say that if someone is throwing hundreds of
thousands of exceptions a minute, they're abusing exceptions - but that
doesn't rule it out nearly as much as many believe.

I think the OP's scheme of returning an exception is a good one - the
exception can be thrown where it's appropriate (eg if log files were
being processed, and those log files really shouldn't contain bad data)
and not where it's not appropriate.

Jon

 

[Guest]

Nick,
True of course on being expensive only when thrown and the bigger picture
concept of a return-value exception from a method being able to be thrown
outside the problem domain.

Also, let's think about working with other developers. Conceptually the idea
seems sound (as Jon pointed out) but what about other developers having to
work with this code - who've never seen anybody do something like this? ( I
haven't - although I've seen an exception being set as a property - not sure
what the intent was though).

Although Daniel should be complemented for his lateral thinking prowess, I'd
still vote for trying first to see if the same effective result could be
accomplished in a more "accepted", best-practices-methodology before
departing into these waters.
Peter
--
Co-founder, Eggheadcafe.com developer portal:
http://www.eggheadcafe.com
UnBlog:
http://petesbloggerama.blogspot.com

Daniel,

I have mixed feelings about this. Also, I think that there needs to be
a correction.

Peter wasn't too clear when he said that exceptions are expensive.
Exceptions are expensive if they are thrown. Creating them, on the other
hand, isn't that expensive. When an exception is created, the stack trace
isn't populated (which is the one thing I can think of which would be
expensive in constructing an exception, but the stack trace is populated
when it is thrown, not when it is created, so it is a non issue).

Now, there is a lot of ingrained thinking about using exceptions like
other objects. We are pretty much given the impression that they are only
to be thrown.

However, they seem to suit your need, and because you are reusing
existing code to suit your needs, it works out well.

I agree with these reasons for using it, and on the surface, it is a
good idea.

However, there is one thing that has been neglected in this discussion
up to this point. That is the fact that whatever you return can be thrown
by any section of code. This is a BIG issue, IMO, as you are giving other
code to really throw something which is specific to your problem domain.
That's just not cool. Granted, in C++, you can throw anything (it doesn't
have to be limited to an object derived from exception), but in most other
..NET languages, you have to derive from exception.

For that reason alone, I think that it is a good idea to create your own
class which will expose the properties you need, which indicates the error
states/information for failed buisiness logic (but not critical errors).

Hope this helps.


--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)

I have had a lively discussion with some coworkers and decided to get
some general feedback on an issue that I could find very little
guidance on. Why is it considered bad practice to define a public
member with a return type that is derived from System.Exception? I
understand the importance of having clean, concise code that follows
widely-accepted patterns and practices, but in this case, I find it
hard to blindly follow a standard when a better, more elegant solution
presents itself so readily.

I was writing a generic function to validate data that is going to be
used to represent key values for my business objects. It looked like
this:

public static ValidationException ValidateKeyArray(System.Type
targetType, object[] keys);

ValidationException is derived from ApplicationException, which, in
turn, derives from the base Exception class. The reason I chose to
write this in this way was to meet three requirements for this method:
a) Validation in itself should not throw an exception unless it cannot
figure out how to validate for some reason.
b) The code calling this method should be able to determine why
validation failed so it can either respond appropriately or throw an
exception if it can't recover
c) Figuring out why validation failed is not a separate logical process
from performing the validation itself, so it makes sense to have a
single function call to do both.

With the function written in this way (returning an object of type
ValidationException) I can:
1) Throw an exception if validation is impossible due to bad input, but
not have to throw an exception if the data is in the correct format but
is invalid
2) Return a descriptive error message in the ValidationException object
3) Return multiple error messages, if necessary (embedded in the
ValidationException object or attached to inner exceptions)
4) Avoid breaking normal program flow when validating, since failed
validation is not an error condition
5) Provide a throwable object to the caller so that critical validation
can be performed in two lines (ValidationException x =
ValidateKeyArray(type, array); if (x != null) throw x

As I understand them, the major alternatives are:

1) Returning a bool. This does not meet requirements (b) and (c).

2) Exposing validation information on the object, like the ASP.NET Page
object does. This would require a Validate method and an IsValid
property, and perhaps some sort of list to provide validation error
messages. This is considerably more code than the one I implemented,
and does not meet requirement (c). Also, validation exceptions would
not be standardized in any way, since I would be relying on the calling
code to throw an exception if it needs to.

3) Always throwing exceptions from the ValidateKeyArray method, and
relying on the caller to use a try...catch block to manage it. If you
know that the input could be bad, shouldn't you avoid throwing an
exception at all?

4) Returning some sort of non-Exception-derived class to expose the
results of validation. The only real disadvantage here is that it
requires some extra code to extract the message from this object and
then put it into an Exception if you need to throw it.

I am looking forward to hearing your thoughts.

Daniel

 

[dcassar]

You can think of the validation method as a sort of exception factory
and it's up to the calling code to determine whether or not there is an
actual error condition, based on its needs. That was my original
intent in structuring my code inthis way. Much like we create business
object managers to centralize access to data and it's up to the calling
code to use them to solve business problems, an exception in itself
describes a potential error state, and it has the useful feature of
being able to be thrown without being wrapped in another class.

I guess I see the problem domain of an exception as not necessarily
local to the method that instantiates it. But, as several people
pointed out, programmers just don't expect to see an exception until
it's being thrown, and I've been coding long enough to know that it's
worth making your intentions clear in the way you structure your code.
So the suggestion of a separate class exposing the validation
information makes lots of sense.

But the problem is that if I have three different methods that call
ValidateKeyArray, they all would be separately responsible for turning
this ValidationResult class into an exception. This would mean that
this new class would then be responsible for generating exceptions if I
want to keep it centralized. A RaiseException method on the
ValidationResult would seem equally as weird as my original solution.
I think you implied at the end of your response that you would separate
handling for failed business logic and true error states, but I don't
know that the ValidationResult class can know one from the other.
Maybe my original method's semantics are wrong?

 

[Alan Pretre]

I'd add one thing to your points. The OP stated he derived his exception
from the ApplicationException class; I feel this is a rather useless
exception type; it does nothing to communicate the cause of an exception,
and it adds depth to the exception hierarchy but adds little of value. I
prefer to derive directly from Exception, or perhaps use a more
descriptive exception type, such as ArgumentException, rather then
ApplicationException or a custom exception.

Being nitpicky...

User exceptions should derive from System.ApplicationException, not
System.Exception. See the Remarks section here in MSDN:
http://msdn.microsoft.com/library/d...frlrfsystemapplicationexceptionclasstopic.asp

To wit:
"This exception is provided as means to differentiate between exceptions
defined by applications versus exceptions defined by the system."

-- Alan

 

[Jon Skeet [C# MVP]]

Being nitpicky...

User exceptions should derive from System.ApplicationException, not
System.Exception. See the Remarks section here in MSDN:
http://msdn.microsoft.com/library/d...frlrfsystemapplicationexceptionclasstopic.asp

To wit:
"This exception is provided as means to differentiate between exceptions
defined by applications versus exceptions defined by the system."

However, look at this:

http://blogs.msdn.com/brada/archive/2004/03/25/96251.aspx
quoting the .NET Framework Standard Library Annotated Reference Vol 1:

<quote>
KC - Designing exception hierarchies is tricky. Well-designed exception
hierarchies are wide, not very deep, and contain only those exceptions
for which there is a programmatic scenario for catching. We added
ApplicationException thinking it would add value by grouping exceptions
declared outside of the .NET Framework, but there is no scenario for
catching ApplicationException and it only adds unnecessary depth to the
hierarchy.

JR - You should not define new exception classes derived from
Application-
Exception; use Exception instead. In addition, you should not write
code that
catches ApplicationException.
</quote>

Jon

 

[Alan Pretre]

<quote>
KC - Designing exception hierarchies is tricky. Well-designed exception
hierarchies are wide, not very deep, and contain only those exceptions
for which there is a programmatic scenario for catching. We added
ApplicationException thinking it would add value by grouping exceptions
declared outside of the .NET Framework, but there is no scenario for
catching ApplicationException and it only adds unnecessary depth to the
hierarchy.

JR - You should not define new exception classes derived from
Application-
Exception; use Exception instead. In addition, you should not write
code that
catches ApplicationException.
</quote>

Hmmm.

Yes, I also see now that rule CA1058 in FxCop discourages deriving from
System.ApplicationException as well. Funny, though, when you follow the
Error Raising and Handling Guidelines link in the FxCop rule, which is

http://msdn.microsoft.com/library/d.../html/cpconerrorraisinghandlingguidelines.asp

it says:

"Do not derive all new exceptions directly from the base class
SystemException. Inherit from SystemException only when creating new
exceptions in System namespaces. Inherit from ApplicationException when
creating new exceptions in other namespaces."


Here is the FxCop rule:

Types should not extend certain base types
TypeName: TypesShouldNotExtendCertainBaseTypes
CheckId: CA1058
Category: Microsoft.Design
Message Level: Error
Certainty: 75%
Breaking Change: Breaking

Cause: An externally visible type extends certain base types. Currently,
this rule reports types that derive from the following types:

a.. System.ApplicationException
b.. System.Xml.XmlDocument

Rule Description
For .NET Framework version 1, it was recommended to derive new exceptions
from ApplicationException. The recommendation has changed and new exceptions
should derive from System.Exception or one of its subclasses in the System
namespace. For more information, see Error Raising and Handling Guidelines

Do not create a subclass of XmlDocument if you want to create an XML view of
an underlying object model or data source.

How to Fix Violations
To fix a violation of this rule, derive the type from a different base type.

When to Exclude Messages
Do not exclude a message from this rule for violations concerning
ApplicationException. It is safe to exclude a message from this rule for
violations about XmlDocument.

See Also
Error Raising and Handling Guidelines



-- Alan

 

[donnie.hale]

Why not return a ValidationResults object rather than a
ValidationException. That could have a couple of key properties:
IsValid; Message; and perhaps ValidationException. So if a client
doesn't like it that a validation failed, it can do "throw
ValidationResults.ValidationException". It's a little like your item
#4, but encapsulates the creation of the exception into the overall
results. This also allows for future enhancements. Perhaps you'll need
a Success, Warning, Error enum at some point. ValidationResults could
encapsulate this as well.

FWIW,

Donnie

 

[kevin cline]

I have had a lively discussion with some coworkers and decided to get
some general feedback on an issue that I could find very little
guidance on. Why is it considered bad practice to define a public
member with a return type that is derived from System.Exception?

Seems perfectly reasonable to me.

I
understand the importance of having clean, concise code that follows
widely-accepted patterns and practices,

Most people rarely see clean, concise code, so when they do see it, it
confuses them.

public static ValidationException ValidateKeyArray(System.Type
targetType, object[] keys);

a) Validation in itself should not throw an exception unless it cannot
figure out how to validate for some reason.
ok...

b) The code calling this method should be able to determine why
validation failed so it can either respond appropriately or throw an
exception if it can't recover

perfectly logical, but I wonder what sort of recovery will be possible?
I would expect that any recovery rules would be closely related to the
validation rules.

c) Figuring out why validation failed is not a separate logical process
from performing the validation itself, so it makes sense to have a
single function call to do both.
True.


With the function written in this way (returning an object of type
ValidationException) I can:
1) Throw an exception if validation is impossible due to bad input, but
not have to throw an exception if the data is in the correct format but
is invalid

It's not at all clear to me the difference between bad input and
invalid input -- they are just different kinds of badness.

2) Return a descriptive error message in the ValidationException object
3) Return multiple error messages, if necessary (embedded in the
ValidationException object or attached to inner exceptions)
4) Avoid breaking normal program flow when validating, since failed
validation is not an error condition

Throwing and catching exceptions allows separation of normal flow from
error flow. Returning a value which must be tested does not.

5) Provide a throwable object to the caller so that critical validation
can be performed in two lines (ValidationException x =
ValidateKeyArray(type, array); if (x != null) throw x

See, you've interrupted normal control flow with that test.

In general, returning values which must be tested by the caller is more
likely to lead to bugs than throwing an exception. If a caller forgets
to test a return value, then execution will proceed as though no error
occurred, even though an error has occurred, and potentially erroneous
results could be returned. But if an exception is uncaught, then
execution will terminate, and someone will know that something went
wrong.

As I understand them, the major alternatives are:

1) Returning a bool. This does not meet requirements (b) and (c).

I agree, this is much worse.

2) Exposing validation information on the object, like the ASP.NET Page
object does. This would require a Validate method and an IsValid
property, and perhaps some sort of list to provide validation error
messages.

Also horrible.

3) Always throwing exceptions from the ValidateKeyArray method, and
relying on the caller to use a try...catch block to manage it. If you
know that the input could be bad, shouldn't you avoid throwing an
exception at all?

I tend to prefer this solution, since it allows better separation of
the normal flow from the error flow. For most applications most of the
time the cost of throwing an exception is negligible. Certainly the
cost of throwing a single exception in a web method is epsilon compared
with the cost of the associated IO.