Today, I ran into a situation where I needed to validate some
Business Rules (BRs) within a business object. To do this, I
needed to
call into some secondary business layers to do some checking, and if a
business rule failed, I needed to note
“Why” and return this result back to the caller. This final caller in
this case was client connecting to a Web-Service endpoint.
My problem arose because I
needed to know two things, IF and WHY the BR failed.
At first glance, I thought I could just throw an exception, and
catch it within my business tier, the exception itself would tell
me the IF, and the exception's message the WHY. I could then
repackage a result object
which would be returned to the web client caller. I asked the
CodeBetter guys what they thought, and was instantly reminded that Rule #1 is not to use exceptions to control program flow.
Raymond Said : You violate the primary rule
of exceptions: do not use exceptions to control the flow of the
program. If you're expecting and exception to occur in that
logic, I would let it propagate back up the call stack. Just my
opinion. Some people do exactly what you are proposing and have
no issues about it. I personally don't like to see them used in
that manner.
In fact, I knew this, but forgot that I had blogged about it over a year ago (and I thought blogs were supposed to help you remember
). The big thing I learned is summed up by these two “almost rules” by Rico Mariani: Exception Cost: When to throw and when not to. (This is actually mentioned in the comments of Jeffrey’s recent post, Why are error return codes evil? - level 100.)
Almost Rule #1
When deciding if you
should throw an exception, pretend that the throw statement makes the
computer beep 3 times, and sleep for 2 seconds. If you still want to
throw under those circumstances, go for it.
Seriously, people
throw much too often, and for things that aren't really very
exceptional at all. In fact, my own opinion is that the framework
itself throws far more often than it should. The actual throwing of
exceptions is sufficiently costly that I try to limit it to cases where
I'm certain that less than 1 call in 1000 (99.9%) to a particular
service will require an exception. This means throwing exceptions on
things like invalid arguments to an API is probably just fine, but on
the other hand throwing an exception due to invalid user input, or
badly formatted text from an external system, could be a bad idea.
Significant use of exceptions in business logic validation is more
often a bad idea than a good one, so be careful out there.
Almost Rule #2
If
you think it will be at all normal for anyone to want to catch your
exception, then probably it shouldn't be an exception at all.
Again,
my favorite consumer of exceptions is a generic exception handler at a
fairly high level on the stack. Anything that fundamentally needs to be
dealt with at a lower level should be looked at suspiciously. Often
such a beast would fail the “only 1/1000” of the time test. Since doing
control flow with exceptions is alot more expensive/confusing than
other forms of control flow its best avoided if the special handling is
commonplace.
What I like to see is general purpose logic for
state cleanup, transaction abort, reset and retry. What I don't like to
see is “downgrading” an exception into a return code, or routinely
being able to “ignore” the exception and proceed. Those latter cases
are usually a bad sign.
After reading this I realized that my original
solution was a bad, bad idea. So, what to do? Remember I
needed to know both the if and the why. Returning two things is
somewhat of a problem, since you can only return one thing from a
method. Jeffrey suggested the following:
Another option is to have the validate
method return a collection of strings of errors. If the
collection is empty, there are no errors.
So, basically an empty versus filled collection would
signify the IF and the collection text the WHY. I asked him if he
thought that this was somewhat of a code smell, since the IF is
inferred, and Jeffrey replied with the following:
It would be a code smell in a
high-level API, but if this was wrapped inside the method that returns
your result object, then it wouldn't be exposed to the outside.
It's really up to you, but in that case, I'd name the method
(GetErrors) or something like that. Then it might be a more clear
that no errors back means you can continue.
Okay, fair enough. But it still not a perfect
solution to this problem, IMO. I thought I’d post about this because
its the kind of real-world coding problems you can run into when you
try to do things the right way. At the end of the day, I really
had to choose the best of a bad set of options frankly – I created a
result object that has a boolean member (succeeded) and a string (extra
message). My business calls return this object. Now, I’m not even
sure if this is the best possible design. Raymond has also
suggested that there may be problems with this – for
example, say you have an error message that indicates that the
method failed with a true value for success. This can be fixed by
making the boolean read only, based on some internal logic (like error
message null) but then the message always has to indicate an
error.
-Brendan