Port of Stubbles IoC to XP #33
Conversation
|
Hm, this shouldn't include the ignore commit... well, ignore the ignores. :) |
|
Nice to see this patch here - thanks! |
…ternal comparing mechanism of lang.Object as it's base implementation not only checks for equality but also for identity which was tried to achieve using spl_object_hash()
|
|
|
Ok, then I'll leave it as it is now. Which in turn brings me to the question of integration into XP. Do we want to cover it within this pull request? Personally I don't think it should be done here - the code is already usable and allows developers to toy around with it. I suggest to gather ideas and handle it with another branch (and pull request), as it takes much more time to develop a working and thought out solution. |
|
No, this pull request should just focus on getting the initial code in, so I agree with @mikey179. We can then take care of integrating this where suitable or toy around with it at the beginning. What I see as the next steps: we'd like to soonish release XP 5.8.2 - our first github release. After this has been released, I'd like to merge this pull request. |
|
Are there any News on this? We are currently reimplementing the Dataset layer, where this would come in handy in a number of situations, along with an integration into the Scriptlet engine. |
|
We plan to include this into HEAD, once 5.8.2 is out. |
There was a problem hiding this comment.
Since we cannot set a name for every method parameter, the @named option (in the implementation above) doesn't really makes sense on methods with more than 1 argument:
#[@inject, @named('???')]
function setTires(Tire $frontLeftTire, Tire $frontRightTire, Tire $backLeftTire, Tire $backRightTire)
Maybe something like this would be more fitting, but I don't find it is very readable:
#[@inject, @named('front-left', 'front-right', 'back-left', 'back-right')]
function setTires(Tire $frontLeftTire, Tire $frontRightTire, Tire $backLeftTire, Tire $backRightTire)
And one other thing that doesn't seem right to me: shouldn't @named be treated as part of the @inject annotation, exactly like the optional= TRUE?
#[@xmlmapping(element= 'tires'), @inject(optional= TRUE, named=['front-left', 'front-right', 'back-left', 'back-right']]
function setTires(Tire $frontLeftTire, Tire $frontRightTire, Tire $backLeftTire, Tire $backRightTire)
There was a problem hiding this comment.
How would it look like if only the last parameter should be named? Something like this?
#[@inject(named=[null, null, 'myapp.config.path'])]
__construct(Processor $proc, Request $req, $configPath)
As there probably will be no parameter annotations in XP it might make sense to treat named as part of the @inject annotation, it just didn't make sense in where it comes from. One might want to move back once parameter annotations become possible, however, I'm undecided on this. What's your take on this, @thekid and @kiesel?
There was a problem hiding this comment.
As we're not adding this before 5.9 we might as well add parameter annotations, too:)
__construct(Processor $proc, Request $req, /*[@inject(named = 'Myapp.config.path')]*/ $configPath)
Not sure what kind of a performance impact these would have.
There was a problem hiding this comment.
Well, if there will be parameter annotations the merge of @inject and @named wouldn't make sense, so this should be decided beforehand then. :-)
There was a problem hiding this comment.
Hm... the example in #116 (method parameter annotations) makes me think about integrating the @named into @inject - having @inject on parameters and not restricted to methods only makes the injector slower and more complicated, as it now must not only check for method annotations, but also for parameter annotations in order to know if the method needs to be called for completing the injection on an instance. I'd rather deny this request now. If you have more than one parameter, and only one is annotated with @inject and the other is not, what's the meaning of that?
…ll by integrating the constant name into the first method which creates the constant binding itself
…ope knows about scriptlet.Session
|
Wiring could be done in classpath initializers, see xp-framework/rfc#220 |
|
Can one of the admins verify this patch? |
Hi,
I just added the port of the Stubbles IoC mechanism to XP. It comes with full unit test coverage, but no further integration into the framework. It may have issues with the XP coding standard, please feel free to point those out.
I do not intend that this pull request gets merged soon (however: feel free if you want to ;), it should be used as a platform to discussion on what should be improved or even how it might be integrated into the framework itself beside being available as code.
Regards,
Frank