Revert "minor #19689 [DI] Cleanup array_key_exists (ro0NL)" #19848
Conversation
|
Tests added, PR ready |
| @@ -223,6 +223,7 @@ public function has($id) | |||
| if ('service_container' === $id | |||
| || isset($this->aliases[$id]) | |||
| || isset($this->services[$id]) | |||
There was a problem hiding this comment.
IIUC, $this->services always exist and is always an array, so this line can be removed (the array_key_exists call in the next will do the check).
There was a problem hiding this comment.
yes but: isset is faster, and isset is what we'll keep on 3.x - and this is a revert, let's stop "optimizing" things that worked fine and had already been reviewed hundred of times.
There was a problem hiding this comment.
Ok to ease the merge between branches.
Regarding the perf, does it really worth to make the code more complex for such micro-optimizations?
There was a problem hiding this comment.
container->has/get() is full of micro-optimizations because it can be called thousands of time.
Which immediately unqualifies this as a "micro"-optimization and makes it a "macro" one.
|
👍 |
…" (nicolas-grekas) This PR was merged into the 2.7 branch. Discussion ---------- Revert "minor #19689 [DI] Cleanup array_key_exists (ro0NL)" | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | Tests pass? | yes | Fixed tickets | #19689 #19840 #19825 #19857 | License | MIT | Doc PR | - This reverts commit c89f80a, reversing changes made to 386e5e7. See discussion in #19847 I'll try adding test cases soon that ensure that: - [x] *when not leaving scope* synthetic services always throw and ignore the `ContainerInterface::NULL_ON_INVALID_REFERENCE` flag (on 3.x also) - [x] *when leaving scope* synthetic services always return null and ignore the `ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE` (until 2.8 since scopes are gone in 3.x) Commits ------- 8cb28bf [DI] Add anti-regression test ac742df Revert "minor #19689 [DI] Cleanup array_key_exists (ro0NL)"
This reverts commit c89f80a, reversing
changes made to 386e5e7.
See discussion in #19847
I'll try adding test cases soon that ensure that:
ContainerInterface::NULL_ON_INVALID_REFERENCEflag (on 3.x also)ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE(until 2.8 since scopes are gone in 3.x)