Using exceptions to break command query separation

I had a conversation with my eight year old daughter this morning about using exceptions as messaging passing.

Me: I hate it when people use exceptions as message passing.
Her: What does that mean?
Me: When you ask someone if something is ok, there are exactly two answers. What are they?
Her: Yes, and no?
Me: That’s right. Using exceptions as message passing would be when instead you either answer nothing, or scream your head off that something has gone terribly wrong.
Her: That’s dumb.

She gets it. My eight year old was able to determine that a yes or no question is not exceptional if the answer is yes or no. Why are there professional developers out there who aren’t able to determine this?

I am tired of looking at code written by professional developers that looks like this:

public void AuthenticateUser(string username, string password)
    string actualPassword = Passwords.ContainsKey(username) ? Passwords[username] : null;

    if (password == null || actualPassword != password)
        throw new ArgumentException("Password doesn't match!");

What is this? When you are asking if a password is valid, or if anything is valid, there are exactly two answers that are expected: yes and no.

Expected answers are not exceptional.

This method should have read like this:

public bool IsUserAuthentic(string username, string password)
    string actualPassword = Passwords.ContainsKey(username) ? Passwords[username] : null;
    return password != null && password == actualPassword;

Now it returns either a yes or a no. The user is either authentic or they aren’t.

Exceptions are great, but they should be saved for situations that you really didn’t see coming or aren’t a valid answer to the question being asked. You want a value from the database, but the database is offline? That could be an exception. You want to access a file on a disk, but file access is locked? That’s an exception. You are dividing by zero? That could be an exception.

Here’s another example:

public void ValidateDateTime(object value)
    DateTime parsedValue = value as DateTime;
    if (parsedValue == null)
        throw new ArgumentException("value wasn't a DateTime!");

This method checks to see if a value is a DateTime. If it is, nothing happens, if it isn’t an exception is thrown.

Here’s the corrected version:

public bool IsValidDateTime(object value)
    return value is DateTime;

The corrected version is even smaller than the version that throws an exception! It should be easier to write.

Part of the problem here is that, this breaks Command Query Separation (CQS). If a method has a void return, then it should be a command. It should have a side effect. These methods do not. They are queries, they just want to know if a user is authentic, or if a value is a valid type. Since they are queries they should have return types, which is why the corrected versions return boolean values.

The other issue with using exceptions in this way is that you are going to litter your code with unnecessary try-catch blocks that could potentially catch actual valid exceptions that you would want to know about. Returning a boolean does not prevent the calling code from throwing an exception if that is, in fact, the correct way to handle it. But throwing an exception means that any code that wants to reuse the logic here will need to wrap this in a try-catch block in order to interact with the result.

You should be writing your code for maximum reuse in order to get as much value from it as you can. In order to do this, CQS and saving exceptions for cases which are actually exceptional will do wonders in the long run.


Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s