Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add callable support to ModelAwareTrait::loadModel() #7060

Closed
wants to merge 1 commit into from

Conversation

jadb
Copy link
Contributor

@jadb jadb commented Jul 19, 2015

This change makes it simpler to create one-off factories by just
doing:

$controller->loadModel(null, ['Cake\ElasticSearch\TypeRegistry', 'get']);

for example or just any anonymous function.

This change makes it simpler to create one-off factories by just
doing `$controller->loadModel(null, [CakeElasticSearchTypeRegistry, get])`
for example or just any anonymous function.
@jadb jadb added this to the 3.1.0 milestone Jul 19, 2015
@markstory
Copy link
Member

Why wouldn't you directly call the factory if you already have a callable and rhe model name?

@jadb
Copy link
Contributor Author

jadb commented Jul 19, 2015

In abstracted libraries (i.e. Crud) you don't. This allows to change:

$this->modelFactory('Type', ['Cake\ElasticSearch\TypeRegistry', 'get']);
$this->Crud->action($this->request->param('action'))->config('modelFactory', 'Type');

by:

$this->Crud->action($this->request->param('action'))
            ->config('modelFactory', ['Cake\ElasticSearch\TypeRegistry', 'get']);

or in some cases, using anonymous functions:

$controller->loadModel(null, function() {
    // do something
    return $repository;
});

@markstory
Copy link
Member

Couldn't those callers know they are getting a callablr and directly invoke it? I guess my hesitation is that in this mode loadModel() is just indirection. The callable isn't used later on or ever again.

@jadb
Copy link
Contributor Author

jadb commented Jul 19, 2015

Hmm, I see the point you are making - don't really have arguments against it. If you guys don't think it should be part of the core, I'm fine with that, just saw an opportunity to write even less code and thought it be suggested ;)

@markstory
Copy link
Member

@jadb I'm not saying its a bad idea, I just don't think adding a wrapper around calling a function is overly valuable.

@markstory markstory closed this Aug 11, 2015
@lorenzo lorenzo deleted the add-callable-support-to-loadmodel branch August 11, 2015 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants