Search code examples
phpfunctionexceptionreturnreturn-value

PHP Exceptions VS Return usage in Functions (best practice)


In our team we often discuss the usage of exceptions inside a function VS some kind of return arrays, but I'm still not sure how to do it the right way. So let's try to ask the question from scratch:

When to use an exception VS returning an array, containing all information about the result?

Example #1

function doAwesomeStuff($foo) {

    // prepare return values...
    $result = [
        'success' => true,
        'infoMessage' => '',
        'stuffDetails' => []
    ];

    // check params...
    if (!is_numeric($foo)) {
        $result['success'] = false;
        $result['infoMessage'] = 'Something went wrong...';
        return $result;
    }

    // do something else...
    if ($somethingElseWentWrong) {
        $result['success'] = false;
        $result['infoMessage'] = 'Something else went wrong...';
        return $result;
    }

    // add new data...
    $result['stuffDetails'] = [
        'foo' => 'bar'
    ];

    // done...
    return $result;
}

$foo = 42;
$awesomeStuff = doAwesomeStuff($foo);
if ($awesomeStuff['success'] === false) {
    echo $awesomeStuff['infoMessage'];
    // some error handling stuff...
}

// do some other awesome stuff
// ...

Example 2

function doAwesomeStuff($foo) {

    // prepare return value...
    $stuffDetails = [];

    // check params...
    if (!is_numeric($foo)) {
        throw new InvalidArgumentException('Something went wrong...');
    }

    // do something else...
    if ($somethingElseWentWrong) {
        throw new AnotherException('Something else went wrong...');
    }

    // add new data...
    $stuffDetails = [
        'foo' => 'bar'
    ];

    // done...
    return $stuffDetails;
}


try {

    $foo = 42;
    $awesomeStuff = doAwesomeStuff($foo);

    // do some other awesome stuff... 
    // ...without caring about error handling at this point (it's all done in catch blocks)

} catch InvalidArgumentException($e) {
    echo $e->getMessage();
    // some error handling stuff...
} catch AnotherException($e) {
    echo $e->getMessage();
    // some error handling stuff...
}

So which version is the "better" way of error handling, #1 or #2? Is it just a matter of taste or are there real arguments for one of the two versions?

There is one simple best practice "rule" for methods or functions which tells you to have single exit points in functions. Is this already the answer, which means it would be v2?

I'd really appreciate all thoughts about this topic :-)


Solution

  • Years ago, when I read about it, I got a lot of information about the problem. First of all, the principle 'Tell, Don't Ask' (here and here) and after the entire Chapter 3 and 7 from Clean Code By Robert C. Martin.

    With this, you can realize some good kinds of stuff to understand what the method should and shouldn't do.

    A method should return only, and just only, when ask or asked for something. Methods asking something has the prefix as:

    • get; //mixed
    • has; //boolean
    • is; //boolean
    • can. //boolean

    or, can has the word return (not necessary as prefix):

    • doSomethingAwesomeAndReturn. //mixed

    The name create is an exception, because create not necessary has to return something:

    • createSomethingAwesomeAndReturn. //mixed

    If your method doesn't have anything of this, it shouldn't return anything.

    Inside the chapter 3, you can find the part called "Prefer Exceptions to Returning Error Codes" and inside chapter 7 you can find "Don’t Return Null". That explains about some problems created from returning something. A lot of validations chain, null object, etc... These problems can be solved just returning an exception.

    Your second example is good, but like the method name says, it does awesome stuff, shouldn't return. If there isn't an exception, it did the awesome stuff. If the return is really necessary, you have to refactor or at least rename your method to match with is expected.

    After all, it's just a lot of suggestion to guide your coding, not an inflexible rule.

    Update

    I've forgotten about the prefix add. It's an exception. A method prefixed as add should return the same object to match with the fluent interface, nothing else. As Fluent Interface at all, it shouldn't match with the above rules doesn't matter the name/prefix/suffix of the method, as it just made for be fluent, not to be cohesive.