I'm pleased to release our first ever round table, where we place a group of developers in a locked room (not really), and ask them to debate one another on a single topic. In this first entry, we discuss exceptions and flow control.
Should Exceptions Ever be Used for Flow Control?
Failure is the inability of a software element to perform its function. Exception is an abnormal condition in software. Errors are due to unmet expectation/specification.
Errors cause Failures and are propagated, via Exceptions.
So:
try { something(); } catch(SomeErrorType e){ return respondTo(e); }
Catching exceptions, as Csaba mentioned, is engineered to be fringe, unexpected cases. These are often a result of malformed input or failed data transfer.
Control flow structures in most languages are optimized to handle known cases, whether that's via an if/else stack or a switch/case. Error throwing, in general, wouldn't be as optimized as control flow in most programming languages.
The client of that code can use a try-catch to prevent the automatic propagation of the exception (as most languages will propagate it). However, I believe that try-catch should be used only for logic that is required to handle these unexpected situations. For example, a broken data transfer can be retried when the exception occurs. Or, some exceptions do not affect the outcome (such as when you want to create a directory that already exists). In this case, it can simply be suppressed.
display_errors
turned off in production.
Aaron, exactly. What you've said does not contradict what I said about catching errors. Thanks for pointing out the more detailed solution.
If the authentication fails, it fails because of fringe cases - bad input, or some problem outside of the actual authentication function. I think, in these instances, we're talking more about a definition of control flow and fringe case than whether or not exceptions are a good way to handle the process of logging in.
This is also an abstraction of the authentication function itself.
An acceptable alternative might be to return a login object that you could run an if/else control over.
$attempt = Sentry::authenticate($credentials); if ($attempt->status == "success"){ $user = $attempt->user; } else if ($attempt->status == "no_password") { // etc }
For example, in the authentication form, if the user name must be an email address and the user types in something else, that could be an exception.
But if the user fills in the form correctly and just the user/pass combination is not a match, that is more likely another case to treat, and not an exception.
if ($attempt->status == "success"){ $user = $attempt->user; } else if ($attempt->status == "failure"){ echo $attempt->message; } else { echo "Something unexpected happened. Please try again."
NodeJS has made a name for itself in this regard, and, for that matter, any "event-driven" runtime, like EventMachine or Twisted.
Csaba, here is a classic async code example:
Let's say that you are trying to read a file: /usr/local/app.log
. The way you do it in Node is:
var fs = require('fs'); // load the filesystem module fs.readFile('/usr/local/app.log', function(err, doc){ if (err) { console.log('failed to read'); return; } // process file });
Because of the callback, you don't put try/catch around the call. Instead, you use a callback style to handle the result. I hope that makes it clear. In general, any operation that cannot be performed synchronously will have a callback style API to handle the results.
.error()
, .success()
, etc. This is an abstraction from what's really going on (checking the XHR object).
$.get("http://example.com/something/remote.json").success(function(data){ // do something with data }).error(function(res){ // do something different; res contains information about the error });
While this isn't either exception or if/else, it still solves the problem: handle both successful and non-successful AJAX requests. However, the jQuery implementation itself is not try/catch, because it is asynchronous.
var auth = require('authenticator'); var eventbus = EventBus.global(); auth.login("pavan", "pwd", function(err, result) { if (err) { var details = { username: "pavan", error: err }; eventbus.put("Authentication failed", details); return; } eventbus.put("Authentication successful!!"); });
Note that we are using the concept of a system-wide eventbus to propagate messages. In this case, we will have a success or failure message with the appropriate payload. This kind of message passing is common in distributed systems and is a great way to have a control flow that spreads across machines.
Other more generally understood cross-boundary control flow is none other than the venerable "Email" and "SMS" messages. It may not be obvious at first glance, but a little introspection and you will see how it is control flow of a different kind, and not done via exceptions.
You can disagree or raise hell in an email, but the receiver is told in a message that may arrive much later than the time it was sent (and may be too late).
try { $conn = connectToDatabase($credentials); } catch (NoDbAtThatUriException $e) { //handle it } catch (LoginException $e) { //handle it }
connectToDatabase
is synchronous, exceptions will work. Otherwise, you need callbacks. Also, there could be several forms of failures (different classes of Exceptions). Do you care what kind of failure it is - especially if you are logging it somewhere?
LoginException
has to be notified to the user?
$e->handleIt()
.
I happened to have picked a bad example, because, in these situations, you basically need to get new data from the user. But, conceptually, you can have full logic, like:
catch (NoDbAtThatUriException $e) { $credentials->uri .= ":3065"; //add port //recall original function here }
Obviously, this isn't the best example and you would probably want to determine if a port is already there, instead of infinitely appending them. But what I'm saying is to use exceptions for actual code, besides simply "alerting" the user.
set_exception_handler()
to handle any uncaught exceptions. You can also have a catch (Exception $e)
block as the last catch statement in the control flow, since all exceptions in PHP extend the native Exception
class.
if
statement is you will get a detailed error if you forget to accommodate one, whereas with an if
statement, it would merely default to the else
.
try { $user = $auth->login($credentials); } catch (InvalidUsernameException $e) { // Redirect to login page with error message // Could even use $e->getMessage() } catch (InvalidPasswordException $e) { // If this is the 5th attempt, redirect to reset password page // Otherwise, redirect to login page with error message } catch (AccountLockedException $e) { // Redirect to special error screen explaining why they aren't allowed } catch (Exception $e) { // Fallback for everything else // Log that we had an unexpected exception // Redirect to error page or something }
Eventually, even if you do decide where it should be handled, debugging is still a problem. If you revisit the code after a few months, you will have little clue what is going on.
The point that I am trying to make is, with control flows of any kind, you are building an implicit state machine and you should strive hard to keep all possible states of control localized in your code. With exceptions, that can be difficult.
return
in your functions to communicate both good and bad scenarios, you may and up with some very unpredictable functions. This is mostly a problem with dynamically-typed languages like PHP, where even the some built-in functions have this problem. For some reason, the people who made them decided that, for example, a function returns a string or an array on success, and false
or -1
on failure.
There has to be a couple exception handlers before it reaches the user, starting from your DB layer to your web layer, and finally rendering it on the client. If you notice the control flow, which is currently spread across many layers, it can be difficult to handle smoothly. What's been your experience in this regard?
As for propagating exceptions between layers. In the project I work on at my job, most exceptions thrown are automatically propagated, and caught very close to the UI, at which point a message is presented to the user.
In other cases, such as creating a directory that already exists, the exception is simply caught by the client code, and discarded with something along the lines of:
try { $filesystemHandler->createDirectory('/tmp/dirname'); } catch (DirectoryExistsException $e) { return true; }
Of course, other exceptions, like NoPermissionToCreateDirectory
will be propagated. I think this is a good example of controlling flow based on an exception.
The user needs to be notified about the progress of the batch process, and eventually alerted when the process is complete. There could be failures, like invalid file, invalid transform, not enough disk space to store the file, etc.
Do you see a control flow here, more like a factory assembly line? How will this be modeled with exceptions, and with regular control constructs?
class ImageTransformer { private $images = []; private $transformer; private $failedTransforms = []; function __construct($images) { $this->images = $images; $this->transformer = new TransformOneImage(); } function transformAll() { foreach ($this->images as $image) { try { $this->transformer->transform($image); } catch (CanNotTranformImageException $e) { $this->failedTransforms[] = $e->getMessage(); } } if (!emptyArray($this->failedTransforms)) { // code to notify user here // and finally return false; } return true; } } class TransformOneImage { function transform($image) { $transformedImage = // do image processing here if (!$transformedImage) { throw new CanNotTranformImageException($image); } return $tranformedImage; } }
The real question here is really about what kinds of cases are considered exceptions. We could easily rewrite this to localize the errors, which would reduce the time needed to identify error source. Of course, with this example, it wouldn't be too difficult to identify a source.
try
statement, and keep in line with the transformations.
When authenticating a user, there essentially two main ways we could have tackled it:
- try/catch
- if/else
Let me explain the ways that we could have tackled if/else:
if (Sentry::authenticate($args)) { // Great, go ahead } else { // Right, something went wrong, but what? }
But, what happens if we want to find out a little more information than a simple "Nope, you're not allowed"? Getting an object back is a great approach:
$response = Sentry::authenticate($args); if ($response->hasError()) { switch ($response->getError()) { case 'login_required': $message = 'You didn\'t enter any login details.'; break; case 'password_required': $message = 'You didn\'t enter a password.'; break; case 'user_not_found': $message = 'No user was found with those credentials.'; break; // And so on... } // Let's pretend we're working in L4 return Redirect::route('login')->withErrors([$message]); } else { return Redirect::route('profile'); }
This has some advantages, primarily due to the switch statement:
if ($response->hasError()) { switch ($response->getError()) { // Consolidate errors case 'login_required': case 'password_required': case 'user_not_found': $message = 'No user was found with those credentials.'; break; // And so on... } return Redirect::route('login')->withErrors([$message]); } else { return Redirect::route('profile'); }
However, exceptions give a lot more control because you can let them be handled at any level within your application. Exceptions can also extend each other, while all extend the base Exception
class (obviously talking PHP here).
A downside to the Exceptions used (particularly with Sentry) is the verbosity of them. This is because we have separated out the different components within Sentry (groups / users / throttling) so that you can take the components you want and build a totally kickass auth system. So, everything that belongs to the 'users' component of Sentry sits in the Cartalyst\Sentry\Users
namespace. A dead-simple way to decrease verbosity is either through the use
keyword: use Cartalyst\Sentry\Users\LoginRequiredException;
. Or, of course, you can go ahead and add a class_alias()
for global aliasing of the class. All of the sudden, we bring the verbosity down to (and with some practical examples):
try { // Set login credentials $credentials = array( 'email' => '[email protected]', 'password' => 'test', ); // Try to authenticate the user $user = Sentry::authenticate($credentials); } catch (LoginRequiredException $e) { // Or a "goto", take your pick return $this->userFail(); } catch (PasswordRequiredException $e) { return $this->userFail(); } catch (UserNotFoundException $e) { return $this->userFail(); } catch (UserNotActivatedException $e) { // Take to a page where the user can resend their activation email return Redirect::to('users/activate'); } catch (UserSuspendedException $e) { return Redirect::to('naughty'); } catch (UserBannedException $e) { return Redirect::to('naughty'); } catch (Exception $e) { // Show a 500 page or something? } return Redirect::route('profile');
Verbosity is one downside to try/catch, but it can be decreased through the use of use
(bad wording there, right?) and class aliases.
Let's consider the positives:
- Logic can be handled at any level of the app or through a custom registered handler (at least in PHP).
- try/catch are "low-level". I mean this in the sense that they don't really change. In PHP, there is always $e->getMessage() and $e->getCode() (due to inheritence from "Exception"). If I return an object to (such as $response->hasError()), the developer needs to know the exposed API for that object. Also, the object may change in the future. try/catch is a syntax which I don't see changing. It's intuitive.
- The only real alternative to having multiple catches (with a catch-all) is switch. But the verbosity of a switch statemtn is much the same as try/catch.
- Mixing true/false and try/catch in the same statement is a recipe for confusion. As @philsturgeon said so well "With a mailer for example, a LOT of things can go wrong in the sending of an email, so you want to throw exceptions if the email fails to contact the SMTP server, if it fails to include a from address, if it cannot find the sendmail install, whatever. What if it doesnt have a valid email address? Is that an exception, or should it return false and make you look up an error code? Why half-and-half it?"
- In PHP, there's no such (real) thing as asynchronous. Before you all jump on my back about spawning processes and all that jank, PHP doesn't really support it. I don't see how using a callback can really improve the application (reminder: I'm talking PHP) or the user experience, as you can achieve the same "progress" feedback (through an app polling your script) throughout the process of whatever happens in the "try" block. 10 points for the worst explanation there, but I think the point still comes across. You can do everything the same through a try/catch as you can using a callback in a single-thread language.
I think, at the end of the day, it comes down to different use-cases. Some may argue that it's the same as "tabs vs spaces," but I don't believe it is. I think there are scenarios when an if/else is appropriate, but, if there is more than one possible non-successful outcome, I believe that a try/catch is usually the best approach.
Here's two more examples that we could discuss:
function create_user($name) { if (strlen($name) === 0) return false; if (user_exists($name)) return false; // This believe it or not returns a user :P return super_magic_create_method($name); } // EXAMPLE ONLY, DON'T SHOOT ME if ($user = create_user($_POST['name'])) { // Hooray } else { // What went wrong?! } function create_user($name) { if (strlen($name) === 0) throw new InvalidArgumentException("You have not provided a valid name."); if (user_exists($name)) throw new RuntimeException("User with name [$name] exists."); // This believe it or not returns a user :P return super_magic_create_method($name); } try { $user = create_user($_POST['name']); } catch (InvalidArgumentException $e) { // Tell the user that some sort of validation failed } // Yep, catch any exception catch (Exception $e) { }
And, for another example, in Laravel 4, there is a validation class. It works like so:
$rules = ['name' => 'required', 'email' => 'required|email']; $validator = Validator::make(Input::get(), $rules); if ($validator->passes()) { // Yay } else { foreach ($validator->errors() as $error) { } }
What if it worked like this?
try { $rules = ['name' => 'required', 'email' => 'required|email']; $validator = Validator::make(Input::get(), $rules); $validator->run(); } catch (ValidationFailedException $e) { foreach ($e->getErrors() as $error) { } } catch (InvalidRulesException $e) { // You made bad rules }
Just food for thought. The first one reads "prettier," but the second one could be seen as optimal. Any thoughts regarding this?
if
statement should now be replaced with an exception. But they are far more "accurate" and readable than, for example. a function that returns -1
or false
.
Perhaps exceptions, then, are best suited for when something goes wrong by definition. For instance, when there is input error, when there is server error, etcetera.
Maybe a bad use-case scenario is catching exceptions for things that are expected and good cases. I certainly agree with what Ben laid out, particularly when it comes to what I have now (un)officially trademarked the Un-informed else block™. (You may affectionately refer to it as UEB™.) This poor catch-all doesn't have anything good to tell us, and unfortunately gets hit with all the bad, with little-to-no recourse. What a shame.
And, thus, we may happen upon an answer! To keep all of our control flow informed, and to use exceptions for things that are, indeed, exceptions.
I should be able to try to log in a user, and know why it failed. Now, the question is, how? I think the answer, as we have all agreed, is that it simply depends.
try { loginUser($creds); } catch (LoginSuccess $success) { //store session and cookies }
So my opinion is to use them for errors. Not limited to "unexpected" errors, but any errors.
Also a point worth adding is that it depends on what you are doing - whether you are performing an action or checking something.
In my Gist above, I had a method validate, and, within the code, I wrote something like:
if ($transform->validate()) { //process } else { //throw exception }
I could have thrown the exception inside the validate method, but that would be destructive to the code's readability. You would have something like:
$transform->validate(); $transform->process();
This makes it unclear what is going on. To summarize my point of view, I would say: use Exceptions when performing an action, and use if
statements when verifying data.
It also works out syntax-wise, because you can say "if (x) then y
", when you are checking data, and try { x } catch (a snag)
for actions. It wouldn't be grammatically correct to interchange them.
- There are multiple points of failure in a particular function/method.
- There is a need to discern the difference between those points of failure and handle them in more than one way.
I think the problem is that some people consider the language an impediment to expressing their ideas. Thus, a mix of if/else with try/catch makes the code harder to understand.
The problem, as I see it, is the exact opposite. The code should reflect the concepts that you want to implement. Using try/catch for everything, except for the happy path, hides a great deal of execution logic details. Anything becomes white or black.
On the other hand, if you use if/else for the cases when your application goes down paths that are part of its logic (like wrong username - passord pair), and then try/catch for the situations when your application gets into an unexpected / unrecoverable state, it would be much more obvious what the possible behavior and paths of execution are.
- Should I ever use JavaScript as a server-side language? No, because that's not what it was designed for.
- Should I use LESS to write CSS? No, because it might confuse someone.
- Is it ever okay to use static methods? No, it is never okay.
All extreme and not constructive.
I think that by presenting valid reasoning from both sides, we reach an even more important conclusion: not condemning other developers for not seeing eye-to-eye with ourselves. If any of you hand me some code, I'm not going to complain about your decision to use exceptions or not. I just want you to be consistent in your approach, and you better comment it! :)
I doubt this will change anything, but most developers could use some more humility and empathy - me especially. That's at least my goal in this: not to convince anyone that my way is the way, or even a better way, but to demonstrate that you shouldn't tell me that my way is flat-out wrong, because maybe we're not looking at it the same way.
It is worth noting why Exceptions are particularly suited for propagating errors. It's all about context and making sure this context is passed on to wherever is best to handle the exception. Most runtimes have an automatic way of bubbling up the exception and that makes it very convenient to inject exception handlers at the right places in the software stack.
You cannot define a bubbling route for exceptions using any language construct. The only possibility is to raise or throw them. It's the runtime's responsibility to do the bubbling. This implicit mechanism makes exceptions the ideal abstraction for propagating errors.
Your Turn
So that's what we have to say on this subject. What are your views? Can an argument be made for using exceptions for flow control?
Comments