individual checks before calling inject* methods
#634
Conversation
1. don't call a method for missing/empty config values 2. check for empty routes in `injectRoutesFromConfig` before checking is_array and running the loop on an empty array (this can be useful if the method is called outside this delegator, which, now, already checks for emptiness before calling it)
| @@ -172,7 +173,7 @@ public static function injectPipelineFromConfig(Application $application, array | |||
| */ | |||
| public static function injectRoutesFromConfig(Application $application, array $config) : void | |||
| { | |||
| if (! isset($config['routes']) || ! is_array($config['routes'])) { | |||
| if (empty($config['routes']) || ! is_array($config['routes'])) { | |||
There was a problem hiding this comment.
I'm not sure about this change. It's a public function and when used directly, you will get an undefined index notice.
There was a problem hiding this comment.
Hello @xtreamwayz , why?
empty acts like ! isset() plus not-empty value. I am just using a stronger checking. I just intercept empty arrays (or whatever) as well...
before: if not set or not array return (does not return early for empty arrays, no need to loop over an empty array)
my-mod: if not set or empty (whatever) or not array return
I am just making the function skip the for loop in case of an empty array
kind regards
There was a problem hiding this comment.
Huh... for some reason I thought it would through a notification but it doesn't. Ignore my comment :)
There was a problem hiding this comment.
@xtreamwayz ,no problem :-)
I was also thinking about storing the resulting locale in a request attribute instead of using Locale::setDefault() in order to make it compatible with async-php setups. What do you reckon?
There was a problem hiding this comment.
@xtreamwayz , oh ...another thing I forgot... the $config var is casted to an array because the inject* methods requires it, so I left it alone...but since I now check for key existence $config should be an array or at least implement ArrayAccess. Usually if a config service exists in a zend-expressive application, it is an array, so we could also remove those argument castings unless we want to support a non array ArrayAccess configuration object.
kind regards
There was a problem hiding this comment.
I was also thinking about storing the resulting locale in a request attribute instead of using Locale::setDefault() in order to make it compatible with async-php setups. What do you reckon?
This is what I do usually. So yes, go ahead.
EDIT: This is for the other PR right?
oh ...another thing I forgot... the $config var is casted to an array because the inject* methods requires it, so I left it alone...but since I now check for key existence $config should be an array or at least implement ArrayAccess. Usually if a config service exists in a zend-expressive application, it is an array, so we could also remove those argument castings unless we want to support a non array ArrayAccess configuration object.
Config should always be an array or ArrayAccess object for Aura.Di.
Usually what we do is $config = $container->get('config') ?? [];.
There was a problem hiding this comment.
@xtreamwayz 1. Oh gosh...yes..right....i have a "few" repos open in so many browser tabs and too little sleep... 2. Good, I am glad I didn't remove the array castings since we need to support ArrayAccess for Aura.Di. best regards.
individual checks before calling `inject*` methods
|
Thanks, @pine3ree. |
This could be considered just as a tiny refactoring / code-style change: no bugs, no new features
inject*method with missing/empty related config valuesinjectRoutesFromConfigbefore checking is_array and running the loop on an empty array (this make more sense if the method is called outside this delegator, which, now, already checks for emptiness before calling it)