10

I am struggling to understand how I should design the error handling parts of my code. I recently asked a similar question about how I should go about returning server error codes to the user, eg. 404 errors. I learnt that I should handle the error from within the current part of the application; seem's simple enough.

However, what should I do when I can't handle the error from the current link in the chain? For example, I may have a class that is used to manage authentication. One of it's methods could be createUser($username, $password). Edit: This method will return a user id or user object. Within that function, I need to determine if the username already exists. If this is true, how should I alert the calling code about this? Returning null instead of a user object is one way. But how do I then know what caused the error?

How should I handle errors in such a way that calling code can easily find out what caused the error? Is there a design pattern commonly used for this kind of situation?

Edit: I forgot to mention: I am using PHP.


Solved: While many people argue that exceptions should not be used in this situation, I have come to the conclusion that it is the best solution.

Firstly, there is no simple, elegant alternative to exceptions. (I recon that it is not possible without a system built into the language... exceptions are already built in.)

Secondly, in response to the "exceptions should only be used for exceptional situations, and this is not one" argument: "When I call getFoo(), I actually expect to get a Foo. If I don't get it, it's by definition an exceptional event." (via, pkainulainen)

Community
  • 1
  • 1
Peter Horne
  • 6,472
  • 7
  • 39
  • 50

7 Answers7

3

There are a few common patterns:

1. Throw an exception.

2. Return NULL or FALSE and set a passed in reference to an error. E.g.

function createUser($user, $password, &$error)
{
      //...
      // You could use any type of object here.  This is just a simple example.
      if(uniqueKeyFailure)
      {
          $error = new UserAlreadyExists();
          return NULL;
      }
      //..
}

It could be called like:

$userCreateError = NULL;
$res = createUser($user, $pass, $userCreateError);
if($res === NULL)
{
  // do something with $userCreateError
}

3. Return NULL or FALSE and provide a get last error (e.g. curl_error).

I would recommend 1 or 2. The main reason people avoid exceptions for "unexceptional" errors like user input is performance. There is a fair amount of discussion on this, such as Performance of try-catch in php.

I do not recommend 3 as it is not reentrant.

Community
  • 1
  • 1
Matthew Flaschen
  • 278,309
  • 50
  • 514
  • 539
  • Could you expand on what you mean by 'set an error object' please? Thanks! – Peter Horne May 11 '10 at 20:37
  • 1
    Of these three, exceptions seem almost tailor-made to deal with the sort of scenarios described in the question. You may pick and choose how to handle each exceptional event based on the class and message of the exception. – AndreiM May 11 '10 at 20:41
  • @tehblanx - I think I disagree. The `createUser` should return `true` on success and `false` otherwise. Ideally this will all be handled in a class and the class itself can handle the issues arising from a failed registration. – thetaiko May 11 '10 at 21:33
  • @thetaiko - I see what you're saying, but I think this is too limiting. 'False' only tells the caller that something went wrong without any further details. What if the design specified that the caller should take one course of action when facing a 'cannot create due to database connection error' vs. a different one for 'cannot create due to duplicate username?' You could implement some sort of error object at that point, but I think exceptions are more elegant for this task. – AndreiM May 12 '10 at 14:22
1

I often handle this kind of thing by sticking my validation inside some domain object (like a user).

For example:

<?PHP
$user->name = 'Joe';
$user->password = 'secret';

//try to save the user.
if (! $user->save()){
   $errors = $user->getMessages();
   // ... do something with error messages
}else{
   //the user object set it's own ID.
   $id = $user->id;
}

Now, a lot of stuff is abstracted behind the user. There might be whole universe of objects in there, but that's up to you.

The bigger issue here is: Why in the world would you have an authentication class create a user? Creation of user objects is probably outside of the scope of Authentication.

It might be sensible to have some method like authenticate($username,$password) return an object of type 'user' (representing the user that just authenticated), but even that is a little messy.

Instead consider something like:

<?PHP
$auth = new AuthService();
$u = new User();
$u->username='Joe';
$u->password='secret';

$authResult = $auth->authenticate($user);

if ($authResult->success){
  //$user has properties set as a side effect.
}else{
  //find out why it failed.
  $errors = $authResult->getErrors();
}

Of course, this requires that you define some kind of result-value class for AuthService::authenticate() to populate and return. But at least you don't end up with methods that sometimes return booleans and sometimes return objects, etc.

timdev
  • 61,857
  • 6
  • 82
  • 92
0

You can return integers that are defined in a global header file with names such as DUPLICATE_USER or INVALID_PASSWORD or whatever is appropriate for your usage. Then the caller function can easily check these return codes against the global header declarations to determine what sort of error occurred.

Eric H
  • 1
  • I forgot to mention, the method will already be returning information (user id or a user object) so I cannot return error codes. – Peter Horne May 11 '10 at 20:38
  • Because once the user has been created I would like to be able to track/use information about the new user. If the method returns a user object then I can go on my way. If it doesn't, then I would have to run a second query to find the user by their username. While that is easy in this situation because the username will be unique, there are many examples where I would need the id to identify the information, eg. storing logs. – Peter Horne May 11 '10 at 20:49
0

This would have to be done in following manner -

Create an error object with error codes in them, error text and the method which threw the error.

Pass the error object back.

You could throw exceptions back but exceptions are expensive

EDIT - Based on the OP's edit. In that case you would have to throw an exception object back.

Create an exception class with error code, error description, class name, method name. Throw the exception back.

Prashant
  • 2,190
  • 4
  • 33
  • 64
0

If you control the return type of the method, you can also change its signature to return something like an OperationResult object that would have an ErrorDescription property and a UserID or User object property.

0

Easiest way: Return NULL if the validation passed, and the error message if it failed. Prevents the need to make Error classes or exceptions. Also the best for performance.

eg.

function numeric($input) {
    if (!is_numeric($input)) return 'Must be numeric.';
    return NULL;
}

function usernameavailable($input) {
    //query database....
    if ($result) return 'Username already taken, please choose another one.';
    return NULL;
}

You can use functions with parameters as well:

function length($input, $min, $max) {
    if (strlen($input) < $min) return "Must be longer than $min characters.";
    if (strlen($input) > $max) return "Must be shorter than $max characters.";
    return NULL;
}

Validating the each element in the entire form is then extremely easy. Just loop over all the elements, call all the validation functions for that element and collect the error strings, if any. If there are no error strings, then all the user input is valid.

I would strongly discourage the suggestions made by one of the other answers. Invalid user input is not Exceptional and does not require an exception or an error. It's simply an expected use case that has to be dealt with in the simplest possible manner.

With this method, you can build a Form class quite easily and simply instantiate elements with the arguments being the validation functions. You can also expand this to include AJAX validation sending asynchronous requests to the exact same PHP validation functions.

Example from my framework:

$Form = new AjaxForm();
$Form->add(new TextBox('Username', 'usernameavailable|length[3,12]'));
$Form->add(new SubmitButton());

With only three lines I've created a fully functional form with both client side and server side validation. Of course all the details are up to you but I still believe that invalid user input should be expected and not be Exceptional.

Lotus Notes
  • 6,302
  • 7
  • 32
  • 47
-2

You are expected to check if the username exists BEFORE calling the createUser() method. If you don't, or something happens between the time you have checked and called createUser(), you throw an exception. It's nowhere near as expensive as it is made out to be.

Does it have a cost? Yes. Is it big enough to matter? Most probably no.

K. Norbert
  • 10,494
  • 5
  • 49
  • 48
  • 1
    It's not about expense, it's about "a user with that name already exists" is not really an exceptional condition. It's actually anticipated that this would happen. Throwing an exception is abuse of exception handling. – timdev May 11 '10 at 20:53
  • that's semantical nonsense! with that kind of reasoning you can explain any error away as "expected" –  May 11 '10 at 20:59
  • If you can anticipate that it can happen, then you check for that case, before trying to create the user, with isUserNameTaken() or something. – K. Norbert May 11 '10 at 21:02
  • No no no no no. Do not "just throw an exception." Exceptions are meant for exceptional circumstances (hence the name). You can EASILY check to see if a name already exists and handle the case appropriately. It's not a special edge case - it's something that could be quite common. – JasCav May 11 '10 at 21:11
  • You are expected to check if the username exists BEFORE calling the createUser() method. If you don't, or something happens between the time you have checked and called createUser(), you throw an exception. – K. Norbert May 11 '10 at 21:21
  • @Jason I agree with your view on the semantics of exceptions, but saying that you can EASILY check is not true. Well, the checking of it is trivial, but the remaining code to then handle the error isn't. Out of all the comments so far no one has put forward a solution that is truely elegant. – Peter Horne May 11 '10 at 23:58
  • You don't do error checking in createUser(). You expect the client code, to check the availability with isUserNameValid(), before calling createUser(). If createUser() fails because of something like a unique constraint on the table, then you throw a SQLException, and let some higher up code deal with it. Having error checking in createUser() is bad, since you cannot check if a username is taken, because it would create the user with the given name in the same go. – K. Norbert May 12 '10 at 08:38
  • @WishCow: Why should I catch an SQLException with higher up code when I can handle the problem lower down, where it is easier to deal with it and provide the user with an informative error. Furthermore, something may happen between the time isUserNameValid() is called and createUser() is called. For this reason, it is wise to leave it until the last possible reason to check for duplicated names. Finally, I, or another developer, may forget to check isUserNameValid() when writting code. Throwing an exception that is not caught will quickly alert me of this error, your method will go undetected. – Peter Horne May 14 '10 at 20:29