Warning BC Break: `data\Model::_classes` and `data\Model_autoConfig` are no more static #746

Merged
merged 1 commit into from Dec 18, 2012

3 participants

@jails
Union of RAD member

Unlike all other variables in data\Model_autoConfig, data\Model::_classes is the only static one. I don't really understand this inconsistency.
Moreover this specificity forces us to redefine all entries in _classes when some custom dependencies is needed.

With with PR you can write:

class MyModel extends \lithium\data\Model {
    protected $_classes = array(
        'custom' => 'my\custom\Dependency'
    );
}

Instead of:

class MyModel extends \lithium\data\Model {
    protected static $_classes = array(
        'connections' => 'lithium\data\Connections',
        'query'       => 'lithium\data\model\Query',
        'validator'   => 'lithium\util\Validator',
        'entity'      => 'lithium\data\Entity'
        'custom' => 'my/custom/Dependency'
    );
}

when you need a custom dependency.

@d1rk
Union of RAD member

What a relief. Had to deal with that several times... Thanks for that.

@nateabele
Union of RAD member

I don't see what being static has to do with whether or not $_classes is properly inherited. It's always been static, and adding dependencies without redefining the whole array definitely used to work. It was probably broken in one of the model initialization refactors.

@jails
Union of RAD member

Sorry, my explanation is confusing. You're right $_classes is properly inherited but only when the instance have been initialized.

e.g.:

public static function test1() {
    print_r(static::$_classes); // display only the overrided dependency
}
public static function test2() {
    static::_object();
    print_r(static::$_classes); // display all the dependencies
}

this means:

  • every time a static function use static::$_classes without a kind of static::_object(); it will fail (unless redefining the whole array).
  • you need to know the internal of the framework to correctly use this static variable.

So for me $_classes should not be static.

@nateabele
Union of RAD member

Fair enough. :-)

@nateabele nateabele merged commit 806ba12 into UnionOfRAD:dev Dec 18, 2012

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment