Search code examples
phpconventions

PHP Let function set a member variable OR let it return the result


I have been wondering about something and I was hoping someone could clarify for me.

Assume we have a class

class Test
{
    private $value;

    function first()
    {
        if("this"=="that") {
            $neededVariable = 1;
        }
    }
    function second()
    {
        $this->first();
        //I need to evaluate something that was done in function first
        if($neededVariable===1) {
            //do something
        }
}

Now what would be the best practice to do? As far as I can tell there are two options:

  1. Let the first() function set the member variable with $this->value = 1; and access that variable with the second() function.

2.The first() function could return $neededVariable.

They both give the same result, but I am wordering if there is a best practice. Maybe one has better perfomance than the other, or simply because it's the convention to go for option 1/2.

Any answers are much appreciated!

Thanks in advance :)


Solution

  • Storing the result of first in $this means modifying the object's state. Using that simply to pass a result is bad design, as a reader of your code will assume that the modification of the object's state has some deeper meaning. Worse, calling second() would also modify the object's state in the same way that first() does - is this something a user of your code would expect?

    If modifying the object's state is the intended purpose of first(), and it would be clear to the user that second() would have the same effect, then setting $this->value in first() and calling first() in second() is good design.

    If you just want to share logic, however, simply write a function that just reuses the logic. That way, you can separate setting state from reusing the logic.

    class Test
    {
        private $value;
    
        private function evaluate() {
          $neededVariable = 0;
          if ("this"=="that") {
             $neededVariable = 1
          }
          return $neededVariable;
        }
    
        function first()
        {
           $this->value = $this->evaluate();
        }
    
        function second()
        {
            $neededVariable = $this->evaluate();
            if($neededVariable===1) {
                //do something
            }
        }
    }