Your opinion please

Hi,

I prefer option #4 :)

As a library method, if Open is going to be used against databases that
could potentially, but not necessarily, require credentials, I think it
should have two overloads: one that takes credentials and one that doesn't.
When credentials are required, the one that doesn't accept them should
thrown an InvalidOperationException. When credentials aren't required but
are supplied anyway, either ignore them or throw an
InvalidOperationException, just to be safe.

You should provide a method or property that allows the calling code to
check whether credentials are required so that it may prompt the user for
them, if necessary, and avoid an exception. A method named,
"RequiresCredentials" or a property named, "CredentialsRequired" sounds
reasonable to me.

--
Dave Sexton

"C# Beginner" <de********@orange.nlwrote in message
news:45***********************@news.wanadoo.nl...

I'm currently creating a database class, which contains a member called
Open(). With this method users can open different databases. When a user
tries to open a database which happens to be secured with useraccounts
(username, password etc), the user should be notifed to login.

Now, in my opinion their are 3 ways to cope with situation:
1. Exit the method nicely with an errorcode (eg access denied, specify
accountinformation and try again)
2. Fire an exception
3. Fire an event (eg a LoginEvent, with cancel option) and wait until the
user specifies his username, password
etc within an exceptionhandler. This means that the Open() must wait
until the user responses.

I'm not looking for a good solution, but I'm more interested in opions
with good arguments.

Is there anybody out there who´s willing to share his opinion?

thnx

DD

Nov 28 '06 #5

Dave Sexton <dave@jwa[remove.this]online.comwrote:

I prefer option #4 :)

As a library method, if Open is going to be used against databases that
could potentially, but not necessarily, require credentials, I think it
should have two overloads: one that takes credentials and one that doesn't.
When credentials are required, the one that doesn't accept them should
thrown an InvalidOperationException. When credentials aren't required but
are supplied anyway, either ignore them or throw an
InvalidOperationException, just to be safe.

You should provide a method or property that allows the calling code to
check whether credentials are required so that it may prompt the user for
them, if necessary, and avoid an exception. A method named,
"RequiresCredentials" or a property named, "CredentialsRequired" sounds
reasonable to me.

This is almost exactly what I was going to post. Special return codes
almost always feel wrong to me, and this feels like a much better
solution to me.

(With regards to the overhead of exceptions, I'd say it should be
regarded as completely irrelevant - the cost of talking to a database
is going to make the overhead of throwing an exception utterly
insignificant. The cost of throwing exceptions should only be seriously
considered when the method in question could reasonably be called
*very* frequently. That doesn't mean that exceptions are always
appropriate to use, of course - but the reasons for not using them
should usually be in terms of appropriate behaviour, not performance.)

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

Nov 28 '06 #6

Hi Jon,

This is almost exactly what I was going to post. Special return codes
almost always feel wrong to me, and this feels like a much better
solution to me.

I certainly agree :)

(With regards to the overhead of exceptions, I'd say it should be
regarded as completely irrelevant - the cost of talking to a database
is going to make the overhead of throwing an exception utterly
insignificant. The cost of throwing exceptions should only be seriously
considered when the method in question could reasonably be called
*very* frequently. That doesn't mean that exceptions are always
appropriate to use, of course - but the reasons for not using them
should usually be in terms of appropriate behaviour, not performance.)

I agree with this as well. The database will certainly be the bottle neck
in almost all situations. And when an exception is thrown due to invalid
state or arguments, as in my suggestion, performance should be of little
concern since you most likely have identified a bug in the calling code.
Fixing it should be the priority and you should be thankful that you threw
an exception instead of using a return code ;)

--
Dave Sexton

Nov 28 '06 #7

C# Beginner

If I interpreted all the responses well, it seems that it is more a question
of how one feels about a certain option. One thing's clear. Nobody is font
of firing events in this situation.

Jon, since you are a MVP, I want to ask you if there is a specific reason
why you feeling wrong about using returncodes (in this situation)? I guess
(but maybe I'm wrong) you are so experienced programming C# that you've
learned to appriceate throwing exceptions and clearly see the benefits
(which I lack because I'm not experienced) of it, or is just a (gut)feeling?

PS: I know my english isn't that great (causing myself getting into
trouble), so I want to make it clear that I'm not trying to offent you any
way !!

DD
"Jon Skeet [C# MVP]" <sk***@pobox.comschreef in bericht
news:MP************************@msnews.microsoft.c om...

Dave Sexton <dave@jwa[remove.this]online.comwrote:
>I prefer option #4 :)

As a library method, if Open is going to be used against databases that
could potentially, but not necessarily, require credentials, I think it
should have two overloads: one that takes credentials and one that
doesn't.
When credentials are required, the one that doesn't accept them should
thrown an InvalidOperationException. When credentials aren't required
but
are supplied anyway, either ignore them or throw an
InvalidOperationException, just to be safe.

You should provide a method or property that allows the calling code to
check whether credentials are required so that it may prompt the user for
them, if necessary, and avoid an exception. A method named,
"RequiresCredentials" or a property named, "CredentialsRequired" sounds
reasonable to me.

This is almost exactly what I was going to post. Special return codes
almost always feel wrong to me, and this feels like a much better
solution to me.

(With regards to the overhead of exceptions, I'd say it should be
regarded as completely irrelevant - the cost of talking to a database
is going to make the overhead of throwing an exception utterly
insignificant. The cost of throwing exceptions should only be seriously
considered when the method in question could reasonably be called
*very* frequently. That doesn't mean that exceptions are always
appropriate to use, of course - but the reasons for not using them
should usually be in terms of appropriate behaviour, not performance.)

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

Nov 29 '06 #8

"Dave Sexton" <dave@jwa[remove.this]online.comwrote:

>You should provide a method or property that allows the calling code to
check whether credentials are required so that it may prompt the user for
them, if necessary, and avoid an exception. A method named,
"RequiresCredentials" or a property named, "CredentialsRequired" sounds
reasonable to me.

I don't know... Think of this code:

bool isreq = db.CredentialsRequired();
if (isreq) db.Login(GetLoginInfo());
db.Access();

What happens if an administrator imposes a security requirement while
this thread is just part way through? Then the db.Access() call will
fail with the access fault. So you're going to need to handle the
access fault in any case.

--
Lucian

Nov 29 '06 #9

Hi Lucian,

>>You should provide a method or property that allows the calling code to
check whether credentials are required so that it may prompt the user for
them, if necessary, and avoid an exception. A method named,
"RequiresCredentials" or a property named, "CredentialsRequired" sounds
reasonable to me.

I don't know... Think of this code:

bool isreq = db.CredentialsRequired();
if (isreq) db.Login(GetLoginInfo());
db.Access();

I envisioned the code to look more like this:

Database db = new Database(connString);

bool secure = db.RequiresCredentials();

// or
// bool secure = db.CredentialsRequired;

if (secure)
db.Open(GetUserCredentials());
else
db.Open();

Where's the problem here?

What happens if an administrator imposes a security requirement while
this thread is just part way through? Then the db.Access() call will
fail with the access fault. So you're going to need to handle the
access fault in any case.

Multi-threading issues are an entirely different thing to consider. I
assume that the Database class will be immutable in that respect. It
doesn't make sense to change whether or not a connection requires
credentials asynchronously, IMO.

--
Dave Sexton

Nov 29 '06 #10

"Dave Sexton" <dave@jwa[remove.this]online.comwrote:

>Multi-threading issues are an entirely different thing to consider. I
assume that the Database class will be immutable in that respect. It
doesn't make sense to change whether or not a connection requires
credentials asynchronously, IMO.

The database will presumably be running as a separate process,
probably even on a separate machine. It's a multi-user database after
all. So asynchronous changes to the database will be an inevitable
fact of life.

--
Lucian

Nov 29 '06 #11

Hi Lucian,

>>Multi-threading issues are an entirely different thing to consider. I
assume that the Database class will be immutable in that respect. It
doesn't make sense to change whether or not a connection requires
credentials asynchronously, IMO.

The database will presumably be running as a separate process,
probably even on a separate machine. It's a multi-user database after
all. So asynchronous changes to the database will be an inevitable
fact of life.

I assumed that you were referring to multi-threading issues that are local
to the Database class itself, but apparently not :)

But regardless of what configuration changes may occur in the DBMS, the Open
method will still be used correctly. What I mean is, the Open method will
be called in a manner that is consistent with the state of the object at the
time it was created, given my assumption of the class's immutability. If
the call should fail due to asynchronous changes to the DBMS, the exception
that will be thrown is certainly appropriate. It doesn't make sense to code
so that an exception is not thrown in that situation, if that's what you're
implying.

This is the same as hard-coding a call to Open() without first checking the
property, which I assume you prefer, and having the DBMS changed to require
credentials instead. An exception should be thrown in both cases, so I'm
not sure why you're against having a property to allow calling code to first
check whether the user should be prompted for credentials.

--
Dave Sexton

Nov 29 '06 #12

ssamuel

@Lucian,

This code block is the same as what you wrote, and the answer to your
question is very simple: if the admin changes the permissions on the
database between the lookup call and the attempt to open the database,
an exception will be thrown. It's an exceptional case, and the classic
scenario for an exception: something external to the program has
changed unexpectedly.

You may make the argument that one may want to expect the admin to
change priviledges frequently. In that case, the requirements for
solving the problem should come with a locking mechanism, having
predicted the need to do something about this without throwing
excessive exceptions.

@else,

A few things to note, variously for the original author and those who
replied:

1) Just because you're implementing a solution that doesn't rely on
exceptions for program flow -- one should avoid that -- it doesn't mean
you shouldn't throw exceptions. Always throw exceptions when something
exceptional happens.

2) I'm a consultant and I've learned that often there isn't a choice 4.
Perhaps irrelevant in this case, business requirements sometimes
require that you pick one of the options, even if it's not your
favorite. Of the choices listed, the first was the best, but I like
your (Jon & Dave) solution much more.

3) There are some reasonable ways to do response codes. Return
enumerations work, as do lightweight objects or structs as return
values. There are advantages and disadvantages to using these,
including readability, performance, complexity, compatibility, and
versatility.

4) For those who claim that I'm incorrect that one should consider
performance of exceptions, I'm sorry to say that you're barking up the
wrong tree. Yes, maybe in this case database calls will take at least
an order of magnitude longer than exception overhead, based on
assumptions you've made that I didn't. Telling a C# beginner that they
don't need to worry about the overhead of exceptions is poor form. I
did not state that performance is the *only* reason that one shouldn't
use exceptions in this case, so there's no reason to go through so much
effort to prove I'm wrong. If it's so true that one only needs worry
about exception performance when something is called "*very*" often,
please tell me *exactly* how often *very* often is in *all* cases so
that we can write it into the C# spec. Until you can do that, consider
re-wording offensive responses to say things like, "the reason one
shouldn't use exceptions in that case is also because it's poor form,
and in that case maybe exception performance doesn't matter as much as
form." You've also fallen into the trap that average programmers and
engineers fall into: assuming that you know exactly how his system
works. How many cycles does it take to do a round-trip database access
on this user's system?
Stephan
Dave Sexton wrote:

Hi Lucian,

>You should provide a method or property that allows the calling code to
check whether credentials are required so that it may prompt the user for
them, if necessary, and avoid an exception. A method named,
"RequiresCredentials" or a property named, "CredentialsRequired" sounds
reasonable to me.
I don't know... Think of this code:

bool isreq = db.CredentialsRequired();
if (isreq) db.Login(GetLoginInfo());
db.Access();

I envisioned the code to look more like this:

Database db = new Database(connString);

bool secure = db.RequiresCredentials();

// or
// bool secure = db.CredentialsRequired;

if (secure)
db.Open(GetUserCredentials());
else
db.Open();

Where's the problem here?

What happens if an administrator imposes a security requirement while
this thread is just part way through? Then the db.Access() call will
fail with the access fault. So you're going to need to handle the
access fault in any case.

Multi-threading issues are an entirely different thing to consider. I
assume that the Database class will be immutable in that respect. It
doesn't make sense to change whether or not a connection requires
credentials asynchronously, IMO.

--
Dave Sexton

Nov 29 '06 #13

C# Beginner <de********@orange.nlwrote:

If I interpreted all the responses well, it seems that it is more a question
of how one feels about a certain option. One thing's clear. Nobody is font
of firing events in this situation.

Jon, since you are a MVP, I want to ask you if there is a specific reason
why you feeling wrong about using returncodes (in this situation)? I guess
(but maybe I'm wrong) you are so experienced programming C# that you've
learned to appriceate throwing exceptions and clearly see the benefits
(which I lack because I'm not experienced) of it, or is just a (gut)feeling?

PS: I know my english isn't that great (causing myself getting into
trouble), so I want to make it clear that I'm not trying to offent you any
way !!

It's very easy to ignore return codes - for example, a lot of people
ignore the return value from calls to Stream.Read, assuming it will
fill the buffer.

Exceptions, however, are much harder to "accidentally" ignore. They can
also be handled more centrally - rather than:

ReturnCode result = FirstOperation();
if (result.IsOkay())
{
result = SecondOperation();
}
if (result.IsOkay())
{
result = ThirdOperation();
}

etc littering your code, you just write:

FirstOperation();
SecondOperation();
ThirdOperation();

possibly putting a try/catch or try/finally round it - although it's
generally much better to have few try/catch statements, handling the
exceptions in a uniform manner from as few handlers as possible. The
semantics of try/finally are much more common, but the "using"
statement tends to make *actual* try/finally blocks rare.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

Nov 29 '06 #14

Lucian Wischik <lu***@wischik.comwrote:

"Dave Sexton" <dave@jwa[remove.this]online.comwrote:
Multi-threading issues are an entirely different thing to consider. I
assume that the Database class will be immutable in that respect. It
doesn't make sense to change whether or not a connection requires
credentials asynchronously, IMO.

The database will presumably be running as a separate process,
probably even on a separate machine. It's a multi-user database after
all. So asynchronous changes to the database will be an inevitable
fact of life.

I don't see that using a return code helps this situation particularly,
unless you deem that if the user manages to open the database
connection without authentication, they're okay from then onwards.

I would suggest that the exception would occur sufficiently rarely
(after all, changing from unauthenticated to authenticated is a pretty
major operation) that in most applications it would be acceptable for
the exception to be caught in a "catch-all" generic handler which just
displayed an error and told the user to start again.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

Nov 29 '06 #15

"Dave Sexton" <dave@jwa[remove.this]online.comwrote:

>An exception should be thrown in both cases, so I'm
not sure why you're against having a property to allow calling code to first
check whether the user should be prompted for credentials.

bool secure = db.RequiresCredentials();
try
{ if (secure)
db.Open(GetUserCredentials());
else
db.Open();
}
catch (ECredentialsRequired e)
{ ...
}

It seems inelegant to have RequiresCredentials-handling code in TWO
places. The person who gets to maintain your code will wonder why you
did it, or forget to update one of the two places. The property as you
described it will ALWAYS have to be used in contexts where the check
gets made twice.
Maybe I'd redesign it a bit. db.Open() always succeeds. If no password
was required at the time of the db.Open() call, then a "session" has
been opened and no password will be required again for the duration of
the session. But if a password was required then the "db" object
remains in a locked state that has to be unlocked. Once a password has
been accepted, the db remains in an unlocked state.

db.Open()
while (db.RequiresCredentials)
{ details = GetLoginDetails();
if (details==cancel) return;
db.AttemptLogin(details);
}
....

Here we handle the "RequiresCredentials" branch in exactly one place.
Subsequent db operations will still throw the ERequiresCredentials
exception if needed. But now the ERequiresCredentials exception will
ONLY EVER be thrown due to programmer errors (i.e. failing to check
the RequiresCredentials property). It will never be checked due to
asynchronous events outside the programmer's control.
--
Lucian

Nov 29 '06 #16

Hi Lucian,

bool secure = db.RequiresCredentials();
try
{ if (secure)
db.Open(GetUserCredentials());
else
db.Open();
}
catch (ECredentialsRequired e)
{ ...
}

It seems inelegant to have RequiresCredentials-handling code in TWO
places. The person who gets to maintain your code will wonder why you
did it, or forget to update one of the two places. The property as you
described it will ALWAYS have to be used in contexts where the check
gets made twice.

You're missing the entire point of the method.

It provides a way for the code to determine whether the user needs to be
prompted, at runtime. Without the method (or property), this would have to
be hard-coded. Since the OP hinted that this code was for a library, the
method (or property) seems perfectly reasonable to me.

Would you prefer the following code instead?

try
{
dbo.Open();
}
catch (ECredentialsRequired e)
{
db.Open(GetUserCredentials());
}

I don't.

<snip>

Your sample is way to complex, IMO.

--
Dave Sexton

Nov 29 '06 #17

ssamuel <ss*****@gmail.comwrote:

4) For those who claim that I'm incorrect that one should consider
performance of exceptions, I'm sorry to say that you're barking up the
wrong tree. Yes, maybe in this case database calls will take at least
an order of magnitude longer than exception overhead, based on
assumptions you've made that I didn't. Telling a C# beginner that they
don't need to worry about the overhead of exceptions is poor form. I
did not state that performance is the *only* reason that one shouldn't
use exceptions in this case, so there's no reason to go through so much
effort to prove I'm wrong. If it's so true that one only needs worry
about exception performance when something is called "*very*" often,
please tell me *exactly* how often *very* often is in *all* cases so
that we can write it into the C# spec. Until you can do that, consider
re-wording offensive responses to say things like, "the reason one
shouldn't use exceptions in that case is also because it's poor form,
and in that case maybe exception performance doesn't matter as much as
form." You've also fallen into the trap that average programmers and
engineers fall into: assuming that you know exactly how his system
works. How many cycles does it take to do a round-trip database access
on this user's system?

The problem is that exceptions are so often taken for granted to be
expensive that very few people think to question this received wisdom.
People see the exception hit when running in a debugger (and that's
significant) and assume that *is* the hit of exceptions - it's not.

While I don't assume I know exactly how his system works, I believe
that it's very, very unlikely that database access is less costly than
the hit of an exception being thrown. I also believe that the cost of a
possible exception is a reasonable price for getting an elegant
solution.

There are *lots* of things which are only to be avoided when they're
going to happen very many times - boxing is a good example. The
difference is that exceptions, when used properly, should almost
*never* end up being thrown thousands of times in a short period of
time.

I see far more examples of people trying to *avoid* exceptions in
perfectly reasonable situations because performance is brought up so
often than I've seen people use exceptions in situations where
performance is a significant reason not to use them.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

Nov 29 '06 #18

"Dave Sexton" <dave@jwa[remove.this]online.comwrote:

>It provides a way for the code to determine whether the user needs to be
prompted, at runtime.

What you wrote merely provides a way for the code to determine whether
the user *NEEDED* to be prompted, past tense, at some earlier point in
time. It doesn't say whether the user needs to be prompted at the time
the Open() call is made.

>Would you prefer the following code instead?

No, I'd prefer the alternative that I did post.

--
Lucian

Nov 29 '06 #19

Lucian Wischik <lu***@wischik.comwrote:

>What you wrote merely provides a way for the code to determine whether
the user *NEEDED* to be prompted, past tense, at some earlier point in
time. It doesn't say whether the user needs to be prompted at the time
the Open() call is made.

Doh. I just realised that the semantics I had assumed for "open" might
as well instead apply to the initial "connect", in which case the
property you described comes out exactly the same as the property I
described. Sorry for the confusion.

--
Lucian

Nov 29 '06 #20