Conditional Logic vs. Exception Handling

[Bullet Magnet]

Join me today for another Q&A session with the inimitable code king, Zorro. This should be a rather simple one.

An issue I've recently stumbled upon and debated is how to handle exceptions in my code. For this example, the user is adding a new category to PassShed, and I don't want a blank category to be entered. Here are the two separate methods I'm debating (in pseudocode):

Method 1

Code: Select all

void submitButtonClickEvent()
{
    if (categoryNameTextBox.Text.Length > 0)
    {
        AddCategory(categoryNameTextBox.Text);
    }
    else
    {
        MessageBox.Show("Blank name.");
    }
}


As you can see here, an exception isn't thrown. The input is simply validated at the GUI level. As I understand it, this could be considered best practice based on the fact that invalid user input should be expected and is therefore not exceptional, i.e. it is not necessary to throw an exception.

Method 2

Code: Select all

void AddCategory(string input)
{
    if (input.Length == 0)
    {
        throw new ArgumentException("Blank name.");
    }
}

void submitButtonClickEvent()
{
    try
    {
        AddCategory(categoryNameTextBox.Text);
    }
    catch (ArgumentException argEx)
    {
        MessageBox.Show(argEx.Message);
    }
}


With this approach, my AddCategory() method handles the exception. The benefits I see here are: 1.) the code in Method 1 would need to be duplicated if I were to enable another way for a category to be added (automatically generated templates, for example). 2.) Given the way the method is defined, wouldn't an incorrectly formatted (null) string parameter be an exceptional error worth catching?

It seems like there are a billion different answers for this, so I'm looking for your opinion on the best practice.

[Bullet Magnet]

Man, I really need to read through some good "clean code" books. I'm slowly realizing just how OCD I've become about organizing my code. I can't stand having to make decisions between two possible ways of doing something.

[BlackCat]

Flip a coin.

[Zorro]

In this particular case, method 1 is the method I'd use. This is debatable, but I always treat exceptions literally. By that I mean that exceptions are "exceptional." Basically, I use exceptions in parts of my stack that may occasionally do things I don't expect or want them to, and catch them accordingly so I can log them and/or attempt an alternative method to achieve the desired result. In the case of forms and user input, we as programmers are already expecting users to occasionally fill out incorrect data, so we code our form error handlers to present those errors to the end user. By definition, user input errors aren't code errors or code exceptions - good software expects bad user input and handles it in a non-exceptional way. Of course this viewpoint is theoretical, and I've seen many developers handle form errors and user input using exception handlers. This practice is not incorrect because the desired results are still achieved; I just don't believe that exceptions are the proper tools for handling user input errors.

Here's a real example in which I've used exceptions (I'm too lazy to convert to pseudo code). It's a snippet from a class used to process credit cards via the Stripe API:

Code: Select all

class Stripe
{
    /**
     * Process a Stripe API Charge method.
     * @param   string  Method: 'create', 'retrieve', 'refund', 'capture'
     * @param   mixed
     * @return  bool
     */
    protected function processCharge($method, $param)
    {
        try {
            switch ($method) {
                // Create a new charge (capture now, or capture later)
                case 'create':
                    $this->parseChargeResponse(Stripe_Charge::create($param));
                    return TRUE;
                    break;
                // Retrieve an existing charge
                case 'retrieve':
                    $this->parseChargeResponse(Stripe_Charge::retrieve($param['id']));
                    return TRUE;
                    break;
                // Refund an existing charge
                case 'refund':
                    $stripe = Stripe_Charge::retrieve($param['id']);
                    $stripe->refund(array('amount' => $param['amount']));
                    $this->parseChargeResponse($stripe);
                    return TRUE;
                    break;
                // Capture a previously uncaptured charge
                case 'capture':
                    $stripe = Stripe_Charge::retrieve($param['id']);
                    $stripe->capture(array('amount' => $param['amount']));
                    $this->parseChargeResponse($stripe);
                    return TRUE;
                    break;
                // Invalid charge method
                default:
                    throw new LogicException('Invalid charge method.');
                    return FALSE;
                break;
            }
        // Card was declined
        } catch (Stripe_CardError $e) {
            $this->setError($e->getMessage());
            return FALSE;
        // Network problem
        } catch (Stripe_ApiConnectionError $e) {
            $this->setError($e->getMessage());
            Vine_Log::logException($e);
            return FALSE;
        // Programming/logic error, must fix
        } catch (Stripe_InvalidRequestError $e) {
            $this->setError($e->getMessage());
            Vine_Log::logException($e);
            return FALSE;
        // Stripe's servers are down
        } catch (Stripe_ApiError $e) {
            $this->setError();
            Vine_Log::logException($e);
            return FALSE;
        // Something else went wrong
        } catch (Exception $e) {
            $this->setError($e->getMessage());
            Vine_Log::logException($e);
            return FALSE;
        }
    }
}