Which design is most preferable: test-create, try-create, create-catch?

Question

Let's assume there is an operation that creates a user. This operation may fail if specified email or username exists. If it has failed, it is required to know exactly why. There are three approaches of doing this as I see it and I'm wondering whether there is a clear winner.

So, here's a class user:

class User
{
    public string Email { get; set; }
    public string UserName { get; set; }
}

And there are 3 ways of accomplishing create operation:

Test-Create

if (UserExists(user)) act on user exists error;
if (UsernameExists(user)) act on username exists error;
CreateUser(user);

UserExists and UsernameExists make request to db server to do a validation. These calls are again repeated in CreateUser to ensure API is used correctly. In case validation has failed, I throw ArgumentOutOfRangeException in both cases. So there is a performance hit.

Try-Create

enum CreateUserResultCode
{
    Success,
    UserAlreadyExists,
    UsernameAlreadyExists
}

if (!TryCreate(user, out resultCode))
{
    switch(resultCode)
    {
        case UserAlreadyExists: act on user exists error;
        case UsernameAlreadyExists: act on username exists error;
    }
}

This pattern does the validation only once, but we resort to using the so called error codes which isn't considered a good practice.

Create-Catch

try
{
    CreateUser(user);
}
catch(UserExistsException)
{
    act on user exists error;
}
catch(UsernameExistsException)
{
    act on username exists error;
}

I don't use error codes here, but I now have to create a separate exception class for every case. It's more or less how exceptions are supposed to be used, but I wonder if creating a separate exception instead of enum entry is worthwhile.

So, do we have a clear winner or it's more a matter of taste?

Answer 1

So, do we have a clear winner or it's more a matter of taste?

The first option has a fundamental flaw - it's never going to be thread safe or safe if CreateUser relies on external resources, and other implementations may create in between your tests. In general, I tend to avoid this "pattern" because of this.

As for the other two options - it really comes down to whether the failure is expected to happen. If CreateUser would be expected to fail on a somewhat normal basis, the Try* pattern is my preference, as using exceptions essentially becomes using exceptions for control flow.

If the failure would truly be an exceptional case, then exceptions would be more understandable.

Answer 2

Test-Create can cause race conditions, so it's not a great idea. It also likely does extra work.

Try-Create is good if you expect errors to be part of normal code flow (such as in the case of user input).

Create-Catch is good if errors are truly exceptional (so you're not worried about performance).

Answer 3

This is somewhat subjective, but there are some concrete pros and cons that are worth pointing out.

One drawback of the test-create approach is the race condition. If two clients attempt to create the same user at approximately the same time, it may be possible for them to both pass the tests, and then attempt to create same user.

Between Try-Create and Create-Catch, I prefer Create-Catch, but that's personal taste. One could argue that Create-Catch uses exceptions for flow control, which is usually frowned upon. On the other hand, Try-Create requires a somewhat awkward output parameter, which could be more easily overlooked.

So, I prefer Create-Catch, but there's definitely room for debate here.

Answer 4

You specified that UserExists and UsernameExists both make DB calls. I'll assume that CreateUser also makes a database call.

Why not let the database handle your threading issues? Assuming further that there are proper constraints set up on your table(s), you can let it err-out within a stored proc call.

Therefore, I'll cast my vote for Create-Catch.