Return a boolean or raise an exception (from a LogOn method)?

[Mr Flibble]

I have a class that has a LogOn method.

I'm trying to think what the advantages are two approaches I have.

a) Having a bool returned containing the success of the LogOn.
b) Raising an exception that should be caught.

I thought that exceptions should not really be raised for "expected"
things and since passing, say, an invalid password to the LogOn routine
is something that is bound to happen sooner or later (and is thus
expected), perhaps it should lead to the LogOn method returning false
rather than true.

Also it affects the client side calling code. Should a client expect an
exception from a LogOn method? If so then the programmer also needs to
be aware that that method is "dangerous" and so needs to put a call to
this method within a try/catch block rather than an if block. But I
guess this should go in the documentation for the class that has the
LogOn method ;-)

Ideas anyone? I've currently done it the option a) way but I'm thinking
I should change it to option b).

Flibble

 

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

Mr Fibble,

I am under the belief that exceptions should be thrown in just that,
exceptional cases. The case of a password being invalid not, in my opinion
an exceptional case, it is, as a matter of fact, a business logic issue, one
where your logic fails.

I would completely expect a return value to be passed back to me
indicating that the login failed (you don't have to make it bool, if you
want to return more information).

Now, if you had a user trying to access a protected resource, then that
is cause for exception, I believe.

Hope this helps.

 

[Vadym Stetsyak]

Hello, Mr!

MF> a) Having a bool returned containing the success of the LogOn.
MF> b) Raising an exception that should be caught.

<skip>

MF> Ideas anyone? I've currently done it the option a) way but I'm thinking
MF> I should change it to option b).

The benefit of exception is that you can shed some light why LogOn failed. OTOH returning simple bool will be of no use.
If you don't want to throw exception then you will have to provide an [out] parameter that will tell why LogOn failed.

I we will search for analogies then...
MS api LogonUser returns BOOL, and if error occurr developer can obtained detailed info via GetLastError()...
However, .NET sec API will throw an exception which Message property will ( probably ) show the cause of error...

--
Regards, Vadym Stetsyak
www: http://vadmyst.blogspot.com

 

[james.curran]

IF you need more information back from the LogOn method then it should
return a enum with one of the possible results. If even more
information is needed, then a LogOnResponse object should be returned.
Throwing an exception is not an appropriate substitute for either of
those.

 

[Helge Jensen]

I have a class that has a LogOn method.

I'm trying to think what the advantages are two approaches I have.

a) Having a bool returned containing the success of the LogOn.
b) Raising an exception that should be caught.

Returning a boolean on such a complicated operation where many things
can go wrong is not really that useful.

You could provide a pair of functions:

void LogOn(out LogonResult result, args...);
void LogOn(args...);

where the first one puts the result of the logon into the out parameter
and the second throws if the LogOn fails.

The second method should be easy to write in terms of the first:

void LogOn(args...) {
LogonResult result;
LogOn(out result, args...);
if ( !result.IsSuccess )
throw new LogonFailure(result);
}

 

[james.curran]

void LogOn(out LogonResult result, args...);
Why have an out parameter? Why not just return the object?

LogonResult LogOn(args);

The one possible problem here is that now both methods have the same
signature, but it also allows the two to be merged:

LogonResult LogOn(args....bool throwOnFailure)
{
LoginResult result= new LogonResult();
/// blah-blah-blah
if (!result.IsSuccess && throwOnFailure)
throw new LogonFailure(result);
return result;

}

 

[Helge Jensen]

Why have an out parameter? Why not just return the object?

For two reasons:

1. The signatures on the two methods would become the same, as you saw

2. The caller cannot erroneously ignore the out-paramter, if he wants
"try-and-exception-on-error" he can use the other LogOn, if he wants
more info he can use the one with the out-pararmeter

LogonResult LogOn(args);

The one possible problem here is that now both methods have the same
signature, but it also allows the two to be merged:

LogonResult LogOn(args....bool throwOnFailure)

Yes, that is another possibility.

Maybe it's just personal preference but I don't think:

LogOn(args..., false);

really indicates what's going on.

And, I don't really like that style too much, it is the first step on
the path to:

f(bool doThis, bool doThat, bool doButnotAlways);

which i've just seen too many times.