exceptions instead of return parameters

[Dan Holmes]

In one of the other threads it was mentioned that using return values
from methods to indicated success/failure of the method should be
replaced with throwing exceptions. Which would mean code like:

List l = object.Find(...);
if (l == null) ... //nothing returned

would be replaced with:

try{
List l = object.Find(...);
....
}
catch (NotFoundException ex)
{
}
catch (Exception ex)
{
}

Is this the preferred method? And what is the background on this?


dan

 

[bombdrop]

It is in my opion better to have a function return a bool for success
or faliure than throwing an exception. Exceptions are resource intesive
so where possible. try something like this

public bool VerifyInitialLogin ()
{
this.emailParamater.Value = this.emailAdress;
this.passwordParamater.Value = this.password1;

//checked To see there is a password1 and email
if (String.IsNullOrEmpty(this.password1) ||
String.IsNullOrEmpty(this.emailAdress)) {
return false;
}

//Get the results back from DB
try {
this.connection.Open();
this.loginDataReader =
this.loginInfoSelectcommand.ExecuteReader();

if (this.loginDataReader.HasRows) {
this.clientID = this.loginDataReader.GetInt16(0);
this.password2 = this.loginDataReader.GetString(3);
this.firstName = this.loginDataReader.GetString(4);
this.sureName = this.loginDataReader.GetString(5);
this.companyName =
this.loginDataReader.GetString(6);
this.isFirstTime =
this.loginDataReader.GetBoolean(7);
if (!IsFirstTime) {
this.reminderQuestion =
this.loginDataReader.GetString(8);
this.reminderQuestionAnswear =
this.loginDataReader.GetString(9);
}
} else {
return false;

}
} catch (MySqlException myEX) {
throw myEX;
} finally {
this.loginDataReader.Close();
this.connection.Close();
}

return true;
}

I only throw an exception if there is some DB error not if there is
some logic error.

 

[VJ]

Throwing custom exceptions are a good habit and is how .NET is built. But
that does not mean, you don't ever return null to indicate something
failed.. Here is how I see it..

Throwing exceptions gives a detail error message of what happened in your
function, if you choose to give that out, if not you can just return null to
indicate you failed in the method by returning null

VJ

 

[Mitchell S. Honnert]

Dan, I haven't read the other thread to which you refer, but I might be able
to help a bit with your question...

In my opinion, I don't think your Find method should throw a
NotFoundException. I know the definition of "failure" is subjective, but in
your example, I don't think the Find method failed to find anything; I think
it *successfully found nothing*. This may sound like it's splitting hairs,
but I do think it illustrates a key distinction.

See, I believe that exceptions should be used when a method fails to perform
its basic function for some reason. So, if the Find couldn't even begin
because a database connection wasn't available or a specified directory path
didn't exist, then an exception should be used. But if the search completed
successfully, but it just so happened that nothing was found, the method did
not fail. In a sense, the search failed, but the search *method* didn't
fail.

The condition of no results being found for a search is a normal outcome of
a Find method. In other words, it's not an exceptional result. Now, if the
search can't even start, now *that's* an exceptional situation and deserves
an exeption. Again, my rule of thumb is to limit exceptions to events that
prevent a method from performing its function, not to convey a normal result
(even if that result is negative).

- Mitchell S. Honnert

 

[Guest]

Thisi s one of those religious debates. Some people believe that code is
more clear when you are returning a value to indicate success or failure.
Whether it is just true or false to indicate success or failure or any of a
bunch of values to indicate exactly what type of failure, it creates a lot of
code in any calling function.

Exceptions allow you to capture an error at whatever level you want to catch
it at without any code between where the problem occurred and where you
process it - for example if you are five function calls deep and want to
handle the error only at the highest level of the call stack - there does not
need to be any code in between. The problems with this approach are that you
generally write more code because once you start throwing exceptions you'll
generally want to start creating your own exception types (which is good but
more work). Also, less experienced coders will think that the only place
they need code is where the error is thrown and where it is caught, but you
will also need error handling in between anywhere you might have alocated any
resources that would need to be dealocated.

Overall, I personally like throwing exceptions, but there is not "one right
way". There's really not even a good concensus.

 

[Guest]

Dan,
A "Not found" condition in a search method is a common and expected business
logic condition, therefore it does not fit the purpose of what exceptions are
for.

Exceptions should be used for really exceptional conditions such as failed
database connection, or the collection itself is null so no search can even
be made.
Peter

 

[Nick Hounsome]

Dan, I haven't read the other thread to which you refer, but I might be
able to help a bit with your question...

In my opinion, I don't think your Find method should throw a
NotFoundException. I know the definition of "failure" is subjective, but
in your example, I don't think the Find method failed to find anything; I
think it *successfully found nothing*. This may sound like it's splitting
hairs, but I do think it illustrates a key distinction.

IMHO a method is called "Find" for one of two reasons:

1) It expects to fail to find the thing sometimes and hence it is not an
abnormal condition and no exception should be thrown.

2) It is not expected to fail to get the object but is stupidly exposing the
implementation detail that a search is involved. In this case it should have
been called "Get".

 

[Nick Hounsome]

Thisi s one of those religious debates. Some people believe that code is
more clear when you are returning a value to indicate success or failure.
Whether it is just true or false to indicate success or failure or any of
a
bunch of values to indicate exactly what type of failure, it creates a lot
of
code in any calling function.

Exceptions allow you to capture an error at whatever level you want to
catch
it at without any code between where the problem occurred and where you
process it - for example if you are five function calls deep and want to
handle the error only at the highest level of the call stack - there does
not
need to be any code in between.

It is incorrect and unhelpful to throw low level exceptions from high level
methods. This usually rules out catching anything like a NotFoundException
from some sort of collection class much above the throw point. Of course you
can catch Exception but only to do something general with it. For example -
if you were to call something like form.ShowDialog() and you got a
NotFoundException you wouldn't know what to do with it which is why methods
such as this do not allow low level exceptions to propagate.

 

[Mark Rae]

I only throw an exception if there is some DB error not if there is
some logic error.

So, if something other than a database error occurs, you don't even care...?

 

[Bruce Wood]

Several of the posts so far argue from the point of view that you are
the writer of all of the code, and so can decide what constitutes an
"exceptional" situation from the point of view of the caller, and what
doesn't.

If that's the case, then exception-versus-return-value is an easy
decision: is it something that your caller is expecting might happen?
Then it's not an exception.

When you're writing a library for future use, however, the waters
become muddier. I wouldn't call it a "religious debate." Rather, I
would call it lack of information about who is going to use your method
and for what. When you don't know who your callers will be, you just
have to make a guess at what is an "exceptional" situation and what is
not. Different people posting here have had different opinions on how
to decide.

You may notice that even in the .NET Framework, sometimes a designer
realizes that the original decision was not the right one. In .NET 1.1,
most primitive types just had a Parse method that threw an exception if
the input string was of the wrong format. Now, in .NET 2.0, we have
TryParse. The original designers decided that "wrong format" was an
exceptional situation, then after seeing how people used the methods,
they decided that "wrong format" was sufficiently common that there
needs to be a way to test it without resorting to exceptions.

The only general rule I would put forth would be that if you expect
your callers to be testing for something a lot, you should offer them a
return value (or another method like TryParse) rather than forcing them
to put try...catch everywhere. try...catch should be an uncommon
construct in client programs, not because "it's slow" (it's not,
terribly), but because it interrupts normal control flow and makes the
code difficult to follow.

 

[Jon Skeet [C# MVP]]

It is in my opion better to have a function return a bool for success
or faliure than throwing an exception. Exceptions are resource intesive
so where possible. try something like this

Just out of interest (and before following the link below) - just how
resource intensive do you think exceptions are? How many exceptions do
you think you could throw per second? How often would you have to throw
them in order to actually see significant performance loss?

A ValidateLogin method probably *should* return a boolean, because its
whole purpose is to verify data. The username and password being
incorrect isn't a failure situation - it's perfectly normal. Any other
error, however - whether database-related or not - should result in
exceptions, IMO.

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

 

[Jon Skeet [C# MVP]]

The only general rule I would put forth would be that if you expect
your callers to be testing for something a lot, you should offer them a
return value (or another method like TryParse) rather than forcing them
to put try...catch everywhere. try...catch should be an uncommon
construct in client programs, not because "it's slow" (it's not,
terribly), but because it interrupts normal control flow and makes the
code difficult to follow.

Spot on. I couldn't have (and plainly haven't!) said it better myself.

 

[Josh Twist]

Great post Dan, I've been meaning to blog on this for a while, so
here's my tuppence.

http://www.thejoyofcode.com/Exceptions.aspx

 

[wfairl]

He's trying to return an object from his find method, not a bool on
whether it's found or not. He needs an "exists" type method on this
class but I definitely think the find method needs to throw an
exception similar to an IndexOutOfRange exception with arrays.
Otherwise there's no way to differientiate between an existing null
item and a non-existent item.

 

[Josh Twist]

Hey wfairl,

I see your point - but my blog entry isn't a direct answer to this post
but a general steam off on the subject.

IMO it all comes back to that implicit assumptions definition I quoted
from Richter.

Your proposition is only valid if there is someway that the things you
are looking for with the Find thing could actually be null, which would
be unusual. The nearest example I can think of is ASP.NET's FindControl
method, this returns null if nothing is found, but then you can't add
Null to the controls collection so that makes sense.

Hopefully Dan's got enough AMMO now to go on and make an informed
decision using his greater knowledge of the requirement and audience.

I beginning to think a *consistent* approach to this particular issue
might be best i.e. always throwing exceptions, but also always provide
a TryGetValue for developers ready to deal with the possibility of a
null return... food for thought anyway. T

 

[Nick Hounsome]

He's trying to return an object from his find method, not a bool on
whether it's found or not. He needs an "exists" type method on this
class but I definitely think the find method needs to throw an
exception similar to an IndexOutOfRange exception with arrays.
Otherwise there's no way to differientiate between an existing null
item and a non-existent item.

There is always "bool Find(object key,out object value)" .
This is probably the most logically correct but unappealing approach.