Is a candidate username that already exists justification to throw an exception?

Question

I have just refactored a lot of code out of my AccountController into an Authentication provider class. The controller used to perform this check:

if (_memberRepository.GetByUserName(model.UserName) != null)
{
    ModelState.AddModelError("", "The user name you have chosen already exists. Please choose another.");
    return View(model);
}

I have moved this into my Authentication class, but there I have no access to ModelState.

if (_memberRepository.GetByUserName(newMember.LoginName) != null)
{
    // Panic stations!!
}

Should I:

a) Return a class that described the status or outcome of the new method? I feel this is overcomplicating things. b) Throw an Exception (maybe ArgumentException?) that signals my objection to registering a duplicate user name? This is quick and simple, yet it borders on using an exception for business logic

I see there is already a MembershipCreateUserException, but I like to steer clear of using the build in membership functionality. It is not good OO and I feel dirty using it.

Also, there is an older question related to yours: http://stackoverflow.com/questions/77127/when-to-throw-an-exception - I'd read some of those responses as well. There are arguments both for and against... — Tieson T., Jul 02 '12 at 06:51

score 2 · Accepted Answer · answered Jul 02 '12 at 06:43

Short answer: input validation should never throw.

Long answer: "The user name you have chosen already exists" is an expected and predictable kind of error, it should be handled gracefully without throwing. It's not different than a wrong password upon login. You should be throwing an exception if you weren't able to create the new user because the database was unreachable, for example.

Nevertheless, you might be able to get things working via exceptions: it's just a matter of best practice.

score 1 · Answer 2 · answered Jul 02 '12 at 06:34

1

You could out an error message. Or, add a simple method to your provider that does nothing but check to see if a name is available (which could then be called asynchronously). Then you've isolated the behavior of name-checking away from your registration logic, as well, so all it has to be concerned with is adding the user.

answered Jul 02 '12 at 06:34

Tieson T.

20,774
6
77
92

Why would I want to do the name checking async? I need a fairly immediate result. – ProfK Jul 02 '12 at 06:36
If this is a web app (which I think it is, since you mentioned the Membership tools), having a way of checking the name as available without requiring a post-back tends to create a better user experience. Round-trips on the web can be time-consuming with a poor connection speed. – Tieson T. Jul 02 '12 at 06:39

score 1 · Answer 3 · answered Jul 02 '12 at 06:41

1

This case is exceptional and I would go with throwing an exception. Checking every time an status class or out parameter seems a lot more dirty to me (and also oldschool). I think its cleaner to write code for the common case and handle exceptional cases with exceptions.

answered Jul 02 '12 at 06:41

Tomas Grosup

6,396
3
30
44

1

a duplicate username upon creating a new user is definitely not an "exception" but an expectable business error. a broken database link while creating would be exceptional... – Alex Jul 02 '12 at 06:44
-1: It is clear from original code that the case is not "exceptional" since it is used in validation of what looks like user input, which clearly expected to have errors. – Alexei Levenkov Jul 02 '12 at 06:46
Thats just about the subjective definition of an exception. If status codes are used, then they need to be propagated to the top and this propagation makes the code more dirty. – Tomas Grosup Jul 02 '12 at 06:54
My reasoning is that you making wrong assumption (or word choice) and build your case on that. Normally cases that are expected are not called "exceptional"... How to handle such cases is open question and throwing exception is acceptable strategy for expected errors if applied consistently through application (as I said I would not do that for my applications). – Alexei Levenkov Jul 02 '12 at 19:57

score 0 · Answer 4 · answered Jul 02 '12 at 06:44

It looks like exception would be wrong since you expect this condition (validation of user input).

I would create separate method that simply check for existance of a user with this name.

You can try to rename existing method to make it clear that some special (null?) value will be returned if user does not exists.

Is a candidate username that already exists justification to throw an exception?

4 Answers4