User tests: Successful: Unsuccessful:
This pull request adds "poor man's" support for dependency injection into the JMenu class tree by utilizing the existing options array to allow class objects to be injected. This helps to identify the class chain's dependencies on other class objects, remove hard couplings to JFactory, and improve testability of the class chain. Also included in this pull request is minor internal structure cleanup and improvements to a couple existing doc blocks.
With the patch applied, a user should be able to navigate the application (site and admin) without issue.
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Category | ⇒ | Libraries |
Easy | No | ⇒ | Yes |
Depends what the approach in J4 is gonna be. If we still end up with a
JFactory like object, this changes nothing. If we go 100% "real" DI, these
identified dependencies would shift to proper constructor arguments. Since
we don't have a factory for building these classes, this approach is the
best compromise we have now to try and do something.
On Thursday, October 8, 2015, George Wilson notifications@github.com
wrote:
Is this worth it when we are actually starting work on J4 and we would
have to add code to work in conjunction with this when we add the 'real' DI?—
Reply to this email directly or view it on GitHub
#8012 (comment).
I am working on it following the way laravel is implementing it, so the application object extends the IoC. Atm I'm still fighting with the autoloading to make sure the old stuff is still working.
Personally I'd shove it in the constructor in J4 and not have to stress about keeping some vague sense of b/c with this then
We're already using this type of DI in JInput to optionally inject a JFilterInput object, so not unprecedented (and never refactored in the Framework). If I'm wasting my time just say so and I'll close it. Not like I don't have other PRs in the same boat.
OK I see we are using it to inject JFilterInput
into JInput
. Fix the conflicts, and I'll do my test.
Done
I have tested this item successfully on 4dfefa8
No issues found
I have tested this item successfully on 4dfefa8
I have tested this item successfully on 4dfefa8
Looks good to me.
Milestone |
Added: |
Milestone |
Added: |
||
Status | Pending | ⇒ | Ready to Commit |
RTC. Thanks
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-10-25 17:02:47 |
Closed_By | ⇒ | roland-d |
Labels |
Removed:
?
|
Thanks everybody. This has now been merged into 3.5-dev.
Is this worth it when we are actually starting work on J4 and we would have to add code to work in conjunction with this when we add the 'real' DI?