1
\$\begingroup\$

I have my own PHP MVC framework that I'm iteratively developing (i.e. adding a feature when I have the time). I'm trying to keep it to the best practices I can, while still adding the most in terms of functionality.

The latest addition I've implemented is lazy loading for my model properties. I've gone, you could say, off the beaten path a bit and decided to make a general Lazy class. It would hold reference to what needs to be instantiated, which is then replaced, on access, but not definition, by its evaluation.

class Lazy extends Object
{
    private $_source;
    private $_class;
    private $_target;
    private $_params;

    function evaluate() {
        if (empty($this->_source)) {
            if (empty($this->_target))
                if (empty($this->_params))
                    return $this->_class->newInstance();
                else return $this->_class->newInstanceArgs($this->_params);
            else {
                if (empty($this->_params))
                    return $this->_target->invoke(null);
                else return $this->_target->invokeArgs(null, $this->_params);
            }
        } else {
            if (empty($this->_target))
                return $this->_source;
            else {
                if (empty($this->_params))
                    return $this->_target->invoke($this->_source);
                else return $this->_target->invokeArgs($this->_source, $this->_params);
            }
        }
    }

    function __construct($source, $target = null, $params = null)
    {
        parent::__construct();

        if (is_object($source))
            $this->_source =& $source;
        $this->_class = new ReflectionClass($source);

        if (!empty($target))
            $this->_target = $this->_class->getMethod($target);

        $this->_params = is_array($params) ? $params : array($params);
    }

    function __destruct()
    {
        parent::__destruct();
    }
}

The 'loading' functionality is serviced by the parent class of the models:

function mapLazy($name, $source, $target = null, $params = null) {
    if (is_array($target)) {
        $params = $target;
        $target = null;
    }

    $this->_lazy[$name] = new Lazy($source, $target, $params);
}

function __get($name)
{
    if (!isset($this->_lazy[$name]))
        throw new Exception_InvalidProperty("No such property found.", 1);

    if ($this->_lazy[$name] instanceof Lazy)
        $this->_lazy[$name] = $this->_lazy[$name]->evaluate();

    return $this->_lazy[$name];
}

Used as such:

$this->mapLazy('Pictures', $this, 'getPictures');
$this->mapLazy('Gallery', 'CMS_Gallery', array(&$this->_gallery_id));

Basically, mapLazy takes a property name to map and creates a hidden instance of a Lazy object and storing the information neccessary for execution in it. Internally, Lazy converts the parameters to the appropriate Reflection children so it can later execute the commands. The reason for all the code in mapLazy and Lazy::ctor is that mapLazy can take a whole lot of parameter forms (which is either a downside or upside of PHP, depends on how you look at it; for this example, overloading would probably work better).

mapLazy('<property>', '<name of class>', '<method / parameters [OPTIONAL]>', '<parameters [OPTIONAL]>');

is the "static" call. Lazy will either instantiate a class (if no second parameter or array of them given), or it will bind to property the result of whatever method was passed in method, possibly with parameters in parameters.

The second major form takes an object instead of class name (ReflectionClass::ctor gracefully doesn't care) and does the same, just on the provided object reference.

After all of that, my question would be one for comments on this implementation of lazy loading and whether or not I'm missing an important flaw or possible improvement. The entire framework can also be found on BitBucket, if desired. Do keep in mind it's mostly an "educational exercise" (although it's used in production on a site), but I'd like to make the best of it.

\$\endgroup\$
  • \$\begingroup\$ Your evaluate() method is going to be a nightmare to debug, it's got really high nPath complexity. Also, evaluate() isn't a great method name, as it tells you very little about what the method actually does. \$\endgroup\$ – GordonM Jul 16 '12 at 10:58
2
\$\begingroup\$

Lazy::evaluate has a lot of logic in it that may be better implemented using inheritance. There seem to be several possibilities:

  • Instantiate a class with or without arguments.
  • Call a function or static method with or without arguments.
  • Call an instance method with or without arguments.
  • Return the $_source value, i.e., not lazy.

I would create multiple subclasses of the abstract class (or interface) Lazy to handle the disparate cases above. As well, I agree that you should define several versions of mapLazy with different names and appropriate type hints. This will provide a nicer API for the developers that must use it.

I don't see an example of when $target is a method. Do you pass in the method's name or its ReflectionMethod? What about functions? Here's where named factory methods would be helpful since they could look up the reflection objects or defer to the lazy subclass.

Remove all those emptys. The following are equivalent to false in a boolean context:

  • false
  • null
  • Empty string
  • Empty array
  • Zero (0 and 0.0)

In PHP 5.0+, variables that hold objects are actually identifiers for looking up the object. Only assign using the reference operator & if you truly need to bind those two variables together. Assigning normally will only copy the identifier and not the original object.

Do you need a destructor that calls the parent's? I suspect that you only need to override it when you want to augment its behavior just like normal methods. However, I admit I've never needed a destructor in PHP and could be mistaken.

This comes down to coding style, but I am not a big fan of leaving off braces for single-line blocks. I used to be, but once I got used to putting them on every time I became a convert. I can't count how many times I've added a second line and forgot to add the braces and wasted time trying to figure out why it was executing the second statement when the block wasn't entered.

Finally, I'd add an explicit public for those methods. I believe this will eventually be required for all methods (no more default) in PHP.

\$\endgroup\$
  • \$\begingroup\$ Whoah, this took me way too long to get back to. Talk about a busy ... year? :) I ended up taking your advice and breaking it into three methods (Instance, Invoke and Static) and refactoring it a bit (incl. removing the &s, heh). I think the API is now indeed much clearer. Thanks for the comments! \$\endgroup\$ – Naltharial Mar 8 '13 at 17:40
  • 1
    \$\begingroup\$ Hey @Naltharial - Would be great if you could post your ultimate solution for others to learn from? \$\endgroup\$ – MikeSchinkel Nov 1 '13 at 0:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.