Returning of new $this with abstract classes

Viewed 74

I found some trouble with my code and do not understand why it's doing as it is. Can anyone explain me?

Let we have:

abstract class AbstractThing
{
    public function search(...)
    {
        $ret = false;

        $data = $database->query(...);
        foreach($data as $values)
        {
            $item  = new $this;
            $item->fill_with_values($values);

            $ret []= $item;
        }

        return $ret;
    }
}

It's works as intended and return object instances at success searches:

class Thing extends AbstractThing
{
    // ...
}

$thing = new Thing;
$things = $thing->search(...); // Thing[] on success, false on failure

But if I wish to shorten code very slightly, it breaks:

abstract class AbstractThing
{
    public function search(...)
    {
        $ret = false;

        $data = $database->query(...);
        foreach($data as $values) {
            $ret []= (new $this)->fill_with_values($values);
        }

        return $ret;
    }
}

This return boolean true. Why? It works well on the classes that are not inherited from abstract class.

3 Answers

The code does 2 different things:

This adds $item to your "$ret" array:

        $item  = new $this;
        $item->fill_with_values($values);

        $ret []= $item;

This adds the returned value of "fill_with_values" to your array:

$ret []= (new $this)->fill_with_values($values);

The equivalent of the above code would be:

        $item  = new $this;
        $return = $item->fill_with_values($values);
        $ret []= $return;

If I knew what was going on in your "fill_with_values" method I could tell you why it is a boolean, but the code does not do the same thing. Hope that makes sense.

When we assign:

$ret []= (new $this)->fill_with_values($values);

...we're not setting $ret[] = (new $this). Instead, this statement pushes the return value of fill_with_values() into the array because it executes last.

It looks like you're trying to implement something similar to the factory method pattern. Consider this:

abstract class AbstractThing
{ 
    ...
    public static function fill($values) 
    { 
        $instance = new static; 
        $instance->fill_with_values($values);

        return $instance; 
    }
}

Then we can actually do what you're trying to accomplish in your question like this:

$ret[] = static::fill($values);

This works because the return value of fill() is the instance of the class, not the return value of fill_with_values(). The static keyword in this context uses late static binding to resolve the type of the class that executes the code (Thing in this case) instead of the class that declares it, so it works through inheritance. See this question for more information.

Ok, finally it was my own error. There was really possibility to return TRUE from fill_with_values() function at some point. Sorry all for bad questions and thanks for answers!

Related