Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Best practice for returning in PHP function/method

I am refactoring an extensive codebase overtime. In the long run we are going to develop the whole system in classes but in the mean time I am using the opportunity to refine my PHP skills and improve some of the legacy code we use across several hundred websites.

I have read conflicting articles over time about how best to return data from a custom function, generally the debate falls into two categories, those concerned about best technical practice and those concerned about ease of reading and presentation.

I am interesting in opinions (with elaboration) on what you consider best practice when returning from a custom PHP function.

I am undecided as to which of the following as a better standard to follow using this basic theoretical function for example;

Approach a.

Populating a return variable and returning it at the end of the function:

<?php
function theoreticalFunction( $var )
{
    $return = '';
    if( $something > $somethingelse ){
       $return = true;
    }else{
       $return = false;
    }
    return $return;
}
?>

Approach b.

Returning at each endpoint:

<?php
function theoreticalFunction( $var )
{
    if( $something > $somethingelse ){
       return true;
    }else{
       return false;
    }
}
?>

A possible duplicate could have been What is the PHP best practice for using functions that return true or false? however this is not limited to simply true or false despite my basic example above.

I have looked through the PSR guidelines but didn't see anything (but I may have missed it so please feel free to point me to PSR with reference :) ).

Extending the original question:

Is the method used to return different depending on the expected/desired output type?

Does this method change depending on the use of procedural or object oriented programming methods? As this question shows, object orientation brings in its own eccentricities to further extend the possible formatting/presentation options Best practices for returns methods in PHP

Please try to be clear in your explanations, I am interested in WHY you choose your preferred method and what, if anything, made you choose it over another method.

like image 390
JohnDevelops Avatar asked May 15 '15 09:05

JohnDevelops


3 Answers

I tend towards early returns - leave the function as soon as you know what is going on. One type of this use if called a 'Guard Clause'

Other things I will often do include dropping final else for a default:

if ($something > $somethingelse) {
   return true;
}
return false;

and in fact, conditions of the form if (boolean) return true; else return false, can be shortened even further (if it is clearer to you) to just return ($something > $somethingelse);. Extracting a complex if clause from code like this to a usefully named function can help clear up the meaning of code a lot.

like image 185
Alister Bulman Avatar answered Oct 25 '22 02:10

Alister Bulman


There are people arguing for single exit points in functions (only one return at the end), and others that argue for fail/return early. It's simply a matter of opinion and readability/comprehensibility on a case-by-case basis. There is hardly any objective technical answer.

The reality is that it's simply not something that can be prescribed dogmatically. Some algorithms are better expressed as A and others work better as B.

In your specific case neither is "best"; your code should be written as:

return $something > $somethingelse;

That would hopefully serve as example that there's simply no such thing as a generally applicable rule.

like image 29
deceze Avatar answered Oct 25 '22 03:10

deceze


I know this question is old but the it is interesting and according to me there are many things to say about it.
The first thing to say is that there is no real standard about returning in functions or methods.
It's usually ruled by the rules your team has decided to follow, but if you are the only one on this refactoring you can do what you think better.

In the case of returning a value the important thing I guess is readability. Sometimes it's better to loose a little bit of performance for a code that is more readable and maintainable.
I will try to show some examples with pros and cons.

Approach A

<?php
function getTariableType($var = null)
{
    if (null === $var) {
        return 0;
    } elseif (is_string($var)) {
        return 1;
    } else {
        return -1;
    }
}

Pros:

  • Explicitness. Each case explains itself, even without any comments.
  • Structure. There is a branch for each case, every case is delimited clearly and it's easy to add a statement for a new case.

Cons:

  • Readability. All these if..else with brackets make the code hard to read and we really have to pay attention to every part to understand.
  • Not required code. The last else statement is not required and the code would be easier to read if the return -1 was only the last statement of the function, outside of any else.

Approach B

<?php
function isTheVariableNull($var)
{
    return (null === $var);
}

Pros:

  • Readability. The code is easy to read and understand, at the first look we know that the function is checking whether the variable is null.
  • Conciseness. There is only one statement and in this case it's fine and clear.

Cons:

  • Limit. This notation is limited to really little funtions. Using this notation or even ternary operator becomes harder to understand in more complicated functions.

Approach C.1

<?php
function doingSomethingIfNotNullAndPositive($var)
{
    if (null !== $var) {
        if (0 < $var) {
            //Doing something
        } else {
            return 0;
        }
    } else {
        return -1;
    }
}

Pros:

  • Explicitness. Each case is explicit we can reconstruct the logic of the function when reading it.

Cons:

  • Readability. When adding many if..else statements the code is really less readable. The code is then indented many times looks dirty. Imagine the code with six nested if.
  • Difficulty to add code. Because the logic seems complex (even if it is not), it's difficult to add code or logic into the function.
  • Plenty of logic. If you have many if..else nested it is perhaps because you should create a second function. NetBeans IDE for example suggests you to create an other function that handles the logic of all your nested blocks. A function should be atomic, it should do only one thing. If it does too much work, has too much logic, it's hard to maintain and understand. Creating an other function may be a good option.

Approach C.2

This approch aims to present an alternative to the C.1 notation.

<?php
function doingSomethingIfNotNullAndPositive($var)
{
    if (null === $var) {
        return -1;
    } elseif (0 >= $var) {
        return 0;
    }
    //Doing something
}

Pros:

  • Readability. This notation is very readable. It's easy to understand what result we will get according to a given value.
  • Explicitness. As C.1, this approach is explicit in every branch of the condition.

Cons:

  • Difficulty to add logic. If the function becomes a bit more complicated, adding logic would be difficult because we may need to move all the branches of the condition.

Approach D

<?php
function kindOfStrlen($var)
{
    $return = -1;
    if (is_string($var)) {
        $return = strlen($var);
    }
    return $return;
}

Pros:

  • Default value. In this structure we can see that the default value is handled from the beginning. We have logic in our function, but if we enter in no branch we have a value anyway.
  • Ease to add logic. If we need to add a branch if it's easy and it does not change the structure of the function.

Const:

  • Not required variable. In this case the $return variable is not required, we would write the same function without using it. The solution would be to return -1 at the end, and return strlen($var) in the if, and it would not be less readable.

Conclusion

I have not listed all the possible notation here, only some of them. What we can think about them is there is no perfect one, but in some cases an approach seems better that an other. For example an is_null function would be fine with the approach B.

Using an approach or an other is really up to you, the important thing is to choose a logic and to keep it during all your project.

like image 36
AnthonyB Avatar answered Oct 25 '22 03:10

AnthonyB