Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Config] Add ConfigurableArrayLoader#21070
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedDec 28, 2016
What's the use case? |
ro0NL commentedDec 28, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Bridging :) without bothering users about implementation details, allowing to write loaders simpply and focus on business value; class MyBaseLoaderextends ConfigurableArrayLoader {protected$target;publicfunction__construct(Domain$target) {parent::__construct(newBusinessConfiguration());$this->target =$target; }}class MySpecialJsonLoaderextends MyBaseLoader {publicfunctionload($file) {returnparent::load(json_decode(file_get_contents($file)); }protectedfunctionloadConfiguration(array$config) {$this->target->setBusiness($config['business']); }} |
nicolas-grekas commentedDec 28, 2016
But users don't write loaders, that's not a business responsibility... |
ro0NL commentedDec 28, 2016
True, but it depends how the user relies on |
ro0NL commentedDec 28, 2016
Anyway, ill have no strong use case myself now. Just trying to make lib code more robust and easier to understand/implement. |
nicolas-grekas commentedDec 28, 2016
Adding more code is not going to make it easier to understand IMHO... |
ro0NL commentedDec 28, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Agree, not per see. Just saying (from a component pov) i could appreciate having this kind of self-bridging classes, doing things once; exposing patterns; exposing business value. |
fabpot commentedDec 28, 2016
I agree with@nicolas-grekas. We never add a feature without a real-world use case. So, I'm closing this PR for now. |
Uh oh!
There was an error while loading.Please reload this page.
Tiny snippet, that bridges
Config\DefinitionwithConfig\Loader, by providing a convenient base classConfigurableArrayLoaderto users (comparable to the currentConfigurableExtension).Would be nice if we considered renaming
FileLoaderLoadExceptiontoLoaderLoadException(or given the package context justLoadExceptionperhaps), to be semantically correct here.