Great, thanks. I left my comments there<http://github.com/castleproject/Castle.InversionOfControl/commit/8b08c71e8ccf591027b39a2c5010e72ec5060d58#commitcomment-93441> .
2010/6/12 Krzysztof Koźmic <[email protected]> > Simone, let me know what you think about this change: > > http://github.com/castleproject/Castle.InversionOfControl/commit/8b08c71e8ccf591027b39a2c5010e72ec5060d58 > > Krzysztof > > > On 14/03/2010 9:19 PM, Simone Busoli wrote: > > It seems we're on a dead end, and I'm pretty sure you didn't use selector > much or you'd get why I'm making these considerations. I'll go on and > implement it myself. > > 2010/3/14 Krzysztof Koźmic <[email protected]> > >> I think you're making a bigger deal out of it, than it really is. >> >> If I3 was there already, it's because other selector, one you injected >> into the container, returned it before. >> You get to steer it by ordering the selectors, which as I stated before >> IMO is enough control. >> Plus do you really have selectors that pick the same interceptors? >> >> I can't think of a good scenario where changing this would be required. >> And IInterceptorSelector is ubiquitous way of filtering/ordering entire set >> of interceptors on a per-method basis. >> >> >> >> On 2010-03-14 00:54, Simone Busoli wrote: >> >> too much unintuitive to rely on the fact that you're storing them in a >> set. I return I1, I2, and I3 but at the end I find the interceptors to be >> I3, I1, I2 because the set already contained I3 so it added I1 and I2 but >> not I3. >> >> 2010/3/13 Krzysztof Koźmic <[email protected]> >> >>> you might use model's interceptors as you wish, but you wouldn't have to. >>> >>> Set by definition does not allow duplicates >>> >>> >>> On 2010-03-13 18:02, Simone Busoli wrote: >>> >>> nope, afaics in the first case you're concatenating all interceptors >>> returned by each selector to the ones returned by the previous selector, >>> which if you merge them with the interceptors in the model each time leads >>> to duplicate interceptors. >>> >>> Maybe you mean something different in the code? >>> >>> 2010/3/13 Krzysztof Koźmic <[email protected]> >>> >>>> Wouldn't it make it identical to the 1st example? >>>> That's what it is doing. >>>> >>>> >>>> On 3/12/2010 11:23 PM, Simone Busoli wrote: >>>> >>>> I like the second one. Any need to pass a second parameter when you >>>> actually pass the component model and this it's writable collection of >>>> interceptors? I would pass only the model and document explicitly not to >>>> modify its collection but instead return the merged collection back. >>>> >>>> 2010/3/12 Krzysztof Koźmic (2) <[email protected]> >>>> >>>>> Order of insertion most likely >>>>> >>>>> On 12 Mar, 09:40, Ayende Rahien <[email protected]> wrote: >>>>> > Yes >>>>> > Although I am not sure what you would be sorting on >>>>> > >>>>> > 2010/3/12 Krzysztof Koźmic (2) <[email protected]> >>>>> > >>>>> > > so we basically have now the following two suggestions (in >>>>> > > pseudocode): >>>>> > >http://gist.github.com/330162 >>>>> > >>>>> > > correct? >>>>> > >>>>> > > On 12 Mar, 09:06, Ayende Rahien <[email protected]> wrote: >>>>> > > > Note; answering the entire thread. >>>>> > >>>>> > > > The original reason behind IModelInterceptorsSelector was that we >>>>> needed >>>>> > > to >>>>> > > > make dynamic decisions about some piecesof code. In particular, >>>>> whatever >>>>> > > > transactions were active or not. >>>>> > > > The design at the time called to allow the >>>>> IModelInterceptorsSelector to >>>>> > > > totally replace the default interceptors, and I would like to >>>>> keep that >>>>> > > as >>>>> > > > an option. >>>>> > > > Regarding the operation mode, I agree that using the return value >>>>> is >>>>> > > clearer >>>>> > > > than a method that is expected to modify its arguments. I would >>>>> actually >>>>> > > go >>>>> > > > further than that and say that we need to pass a readonly >>>>> collection to >>>>> > > that >>>>> > > > method. Not a modifable one. >>>>> > > > When the docs say that you merge the model.Interceptors with what >>>>> the >>>>> > > > selector returns, it means that you merge them and _return the >>>>> merged >>>>> > > > value_. >>>>> > >>>>> > > > 2010/3/11 Simone Busoli <[email protected]> >>>>> > >>>>> > > > > sure, I will prepare one for a real case I'm on now. >>>>> > >>>>> > > > > 2010/3/11 Krzysztof Koźmic <[email protected]> >>>>> > >>>>> > > > >> the way I understand it you want to create a single empty >>>>> collection >>>>> > > of >>>>> > > > >> interceptors, and pass it to each selector in turn. >>>>> > > > >> This means that subsequent selectors can see, and override >>>>> choice made >>>>> > > by >>>>> > > > >> previous one, which breaks independence of selectors. >>>>> > >>>>> > > > >> I really feel we should discuss a real example at this point. >>>>> > >>>>> > > > >> Krzysztof >>>>> > >>>>> > > > >> On 3/11/2010 8:16 AM, Simone Busoli wrote: >>>>> > >>>>> > > > >> I think all your considerations are well satisfied by a void >>>>> return >>>>> > > method >>>>> > > > >> which modifies the existing collection in any way it wants. >>>>> > > > >> If you don't like modifying method arguments then we could >>>>> pass in a >>>>> > > > >> delegate. >>>>> > >>>>> > > > >> 2010/3/11 Krzysztof Koźmic <[email protected]> >>>>> > >>>>> > > > >>> we've been relying on order to do decorators, with quite good >>>>> > > results. >>>>> > > > >>> We're cutting edge here, we give people scissors. >>>>> > >>>>> > > > >>> Current design keeps selectors independent, which I consider >>>>> to be a >>>>> > > good >>>>> > > > >>> thing. >>>>> > > > >>> You don't have to have just one selector if you need so. I >>>>> expressed >>>>> > > my >>>>> > > > >>> personal preference, nothing more. I would use more than a >>>>> single >>>>> > > selector >>>>> > > > >>> if I felt a need with no hesitation. >>>>> > > > >>> Also current design does not put any requirements at you WRT >>>>> > > > >>> model.Interceptors. >>>>> > >>>>> > > > >>> Perhaps it would be more beneficial if we moved from abstract >>>>> concept >>>>> > > to >>>>> > > > >>> some actual example where you feel having that capability >>>>> (single jar >>>>> > > of >>>>> > > > >>> interceptors passed between all selectors) would be required. >>>>> > >>>>> > > > >>> cheers, >>>>> > > > >>> Krzysztof >>>>> > >>>>> > > > >>> On 3/10/2010 10:53 PM, Simone Busoli wrote: >>>>> > >>>>> > > > >>> I see selectors as a step in the pipeline where you can >>>>> apply >>>>> > > > >>> additional concerns, you don't care about what's already >>>>> there, you >>>>> > > just add >>>>> > > > >>> behavior if necessary. >>>>> > > > >>> That's why I don't think there should be only 1 selector and >>>>> why a >>>>> > > > >>> selector shouldn't care about the stuff that's already in >>>>> > > > >>> model.Interceptors. That's why I'm saying we're probably >>>>> looking at >>>>> > > things >>>>> > > > >>> under a different perspective. >>>>> > >>>>> > > > >>> About order of registration for selectors, I think it's a >>>>> very bad >>>>> > > way >>>>> > > > >>> of doing things. To me the order of two calls to >>>>> container.Register >>>>> > > (or most >>>>> > > > >>> of what else you could put into the container: facilities, >>>>> > > interceptors,...) >>>>> > > > >>> should mean as much as the order of two fields in a class. >>>>> > > > >>> If you're relying on that you're looking for troubles. >>>>> > >>>>> > > > >>> 2010/3/10 Krzysztof Koźmic <[email protected]> >>>>> > >>>>> > > > >>>> On 3/10/2010 10:37 PM, Simone Busoli wrote: >>>>> > >>>>> > > > >>>> inline >>>>> > >>>>> > > > >>>> 2010/3/10 Krzysztof Koźmic <[email protected]> >>>>> > >>>>> > > > >>>>> On 3/10/2010 10:15 PM, Simone Busoli wrote: >>>>> > >>>>> > > > >>>>> 2010/3/10 Krzysztof Koźmic <[email protected]> >>>>> > >>>>> > > > >>>>>> On 3/10/2010 9:59 PM, Simone Busoli wrote: >>>>> > >>>>> > > > >>>>>> inline >>>>> > >>>>> > > > >>>>>> 2010/3/10 Krzysztof Koźmic <[email protected]> >>>>> > >>>>> > > > >>>>>>> you put interceptors in whatever order you please. >>>>> > >>>>> > > > >>>>>> how? >>>>> > >>>>> > > > >>>>>> You create the array yourself in the selector. You choose >>>>> what to >>>>> > > put >>>>> > > > >>>>>> there, and in what order. it doesn't even have to have >>>>> anything in >>>>> > > common >>>>> > > > >>>>>> with componentModel.Interceptors collection >>>>> > >>>>> > > > >>>>> Can you see that all interceptors in model.Interceptors >>>>> are >>>>> > > appended >>>>> > > > >>>>> after those selected by all selectors? >>>>> > >>>>> > > > >>>>> Yes, I can see that. I would execute that code only when >>>>> no >>>>> > > selector >>>>> > > > >>>>> has opinion about component model in question. >>>>> > >>>>> > > > >>>> I'm not sure that would be a good choice, but we definitely >>>>> need to >>>>> > > > >>>> define what selectors are for. In my opinion they should >>>>> have a >>>>> > > chance to >>>>> > > > >>>> modify the existing collection of interceptors. If they >>>>> don't care >>>>> > > about the >>>>> > > > >>>> order then they will eventually append their stuff otherwise >>>>> they >>>>> > > will do >>>>> > > > >>>> nothing. With the current code we're really looking for bugs >>>>> with >>>>> > > duplicate >>>>> > > > >>>> interceptors appended at the end. >>>>> > >>>>> > > > >>>> Why wouldn't this be a good choice? >>>>> > >>>>> > > > >>>> Either selectors want to override the choice, or we go with >>>>> default. >>>>> > > > >>>> Should work IMO. >>>>> > >>>>> > > > >>>>>>> between selectors order of selectors transfers to order >>>>> of >>>>> > > > >>>>>>> interceptors. Plus I think you should not have multiple >>>>> selectors >>>>> > > for >>>>> > > > >>>>>>> one model so it's a non-issue anyway. >>>>> > >>>>> > > > >>>>>> why? I'm using multiple selectors to apply different and >>>>> > > unrelated >>>>> > > > >>>>>> concerns to models. Do you think I should centralize a >>>>> bunch of >>>>> > > unrelated >>>>> > > > >>>>>> stuff into the same class? >>>>> > >>>>> > > > >>>>>> Give me a scenario. But despite of it, doesn't order of >>>>> selectors >>>>> > > > >>>>>> give you enough control? >>>>> > >>>>> > > > >>>>> A selector handling whether we need to add transaction >>>>> interceptor >>>>> > > > >>>>> and another handling exception-related stuff. >>>>> > > > >>>>> Ideally I wouldn't like to depend on the order of selectors >>>>> and put >>>>> > > > >>>>> that logic in the selectors themselves, for instance, >>>>> transaction >>>>> > > > >>>>> interceptor should be last. (in this I would like to extend >>>>> > > > >>>>> InterceptorReferenceCollection to support keeping an >>>>> interceptor in >>>>> > > a >>>>> > > > >>>>> certain position even if other interceptors are added >>>>> later) >>>>> > >>>>> > > > >>>>> now we're talking :) >>>>> > > > >>>>> I still think that you can achieve this with ordering of >>>>> selectors >>>>> > > > >>>>> without changing the interface. Plus you always can use >>>>> > > IInterceptorSelector >>>>> > > > >>>>> to select/order interceptors at a method level. >>>>> > >>>>> > > > >>>> Would you be confident in relying on the order in which you >>>>> > > register >>>>> > > > >>>> your selectors? I'd prefer to let the selector itself decide >>>>> where >>>>> > > to put >>>>> > > > >>>> its interceptor/s. >>>>> > >>>>> > > > >>>> I think I would 99% of the time. And for the remaining 1% >>>>> I'd use >>>>> > > > >>>> IInterceptorSelector. >>>>> > >>>>> > > > >>>>>>> in addition I'm against passing interceptors selected by >>>>> one >>>>> > > > >>>>>>> selector, >>>>> > > > >>>>>>> to subsequent selectors. >>>>> > >>>>> > > > >>>>>> this is the current code. how do you apply order? how do >>>>> you >>>>> > > remove >>>>> > > > >>>>>> interceptors? >>>>> > >>>>> > > > >>>>>> foreach(IModelInterceptorsSelector selector in >>>>> selectors)// >>>>> > > > >>>>>> selectors are asked in order you register them in >>>>> > > > >>>>>> { >>>>> > > > >>>>>> InterceptorReference[] interceptors = >>>>> > > > >>>>>> selector.SelectInterceptors(model); >>>>> > >>>>> > > > >>>>>> + if (interceptors == null) >>>>> > > > >>>>>> + { >>>>> > > > >>>>>> + continue; >>>>> > > > >>>>>> + } >>>>> > > > >>>>>> + >>>>> > > > >>>>>> + foreach (InterceptorReference interceptor in >>>>> interceptors) >>>>> > > > >>>>>> + yield return interceptor; // interceptors are returned >>>>> in order >>>>> > > > >>>>>> selector put them in the array >>>>> > > > >>>>>> } >>>>> > > > >>>>>> + >>>>> > > > >>>>>> + foreach (InterceptorReference interceptor in >>>>> model.Interceptors) >>>>> > > > >>>>>> + yield return interceptor; >>>>> > > > >>>>>> } >>>>> > >>>>> > > > >>>>>> Note that model.Interceptors are concatenated to anything >>>>> > > returned >>>>> > > > >>>>> by selectors, so you don't have control unless you modify >>>>> the >>>>> > > collection >>>>> > > > >>>>> directly. >>>>> > >>>>> > > > >>>>>>> 2010/3/10 Simone Busoli <[email protected]>: >>>>> > > > >>>>>>> > Except that I don't agree with this principle, the >>>>> return >>>>> > > value >>>>> > > > >>>>>>> doesn't let >>>>> > > > >>>>>>> > you specify where exactly to put the interceptor. So >>>>> the return >>>>> > > > >>>>>>> value >>>>> > > > >>>>>>> > provides a subset of the functionality provided by the >>>>> input >>>>> > > > >>>>>>> collection. >>>>> > >>>>> > > > >>>>>>> > 2010/3/10 Krzysztof Koźmic <[email protected] >>>>> > >>>>> > >>>>> > > > >>>>>>> >> void methods should not modify their arguments >>>>> > >>>>> > > > >>>>>>> > -- >>>>> > > > >>>>>>> > You received this message because you are subscribed >>>>> to the >>>>> > > > >>>>>>> Google Groups >>>>> > > > >>>>>>> > "Castle Project Users" group. >>>>> > > > >>>>>>> > To post to this group, send email to >>>>> > > > >>>>>>> [email protected]. >>>>> > > > >>>>>>> > To unsubscribe from this group, send email to >>>>> > > > >>>>>>> > [email protected]<castle-project-users%[email protected]> >>>>> <castle-project-users%[email protected]<castle-project-users%[email protected]> >>>>> > >>>>> > > <castle-project-users%[email protected]<castle-project-users%[email protected]> >>>>> <castle-project-users%[email protected]<castle-project-users%[email protected]> >>>>> > >>>>> > >>>>> > > > >>>>>>> . >>>>> > > > >>>>>>> > For more options, visit this group at >>>>> > > > >>>>>>> > >>>>> http://groups.google.com/group/castle-project-users?hl=en. >>>>> > >>>>> > > > >>>>>>> -- >>>>> > > > >>>>>>> You received this message because you are subscribed to >>>>> the >>>>> > > Google >>>>> > > > >>>>>>> Groups "Castle Project Users" group. >>>>> > > > >>>>>>> To post to this group, send email to >>>>> > > > >>>>>>> [email protected]. >>>>> > > > >>>>>>> To unsubscribe from this group, send email to >>>>> > > > >>>>>>> [email protected]<castle-project-users%[email protected]> >>>>> <castle-project-users%[email protected]<castle-project-users%[email protected]> >>>>> > >>>>> > >>>>> > ... >>>>> > >>>>> > więcej >> >>>>> >>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "Castle Project Users" group. >>>>> To post to this group, send email to >>>>> [email protected]. >>>>> To unsubscribe from this group, send email to >>>>> [email protected]<castle-project-users%[email protected]> >>>>> . >>>>> For more options, visit this group at >>>>> http://groups.google.com/group/castle-project-users?hl=en. >>>>> >>>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "Castle Project Users" group. >>>> To post to this group, send email to >>>> [email protected]. >>>> To unsubscribe from this group, send email to >>>> [email protected]. >>>> For more options, visit this group at >>>> http://groups.google.com/group/castle-project-users?hl=en. >>>> >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "Castle Project Users" group. >>>> To post to this group, send email to >>>> [email protected]. >>>> To unsubscribe from this group, send email to >>>> [email protected]<castle-project-users%[email protected]> >>>> . >>>> For more options, visit this group at >>>> http://groups.google.com/group/castle-project-users?hl=en. >>>> >>> >>> -- >>> You received this message because you are subscribed to the Google Groups >>> "Castle Project Users" group. >>> To post to this group, send email to >>> [email protected]. >>> To unsubscribe from this group, send email to >>> [email protected]. >>> For more options, visit this group at >>> http://groups.google.com/group/castle-project-users?hl=en. >>> >>> >>> -- >>> You received this message because you are subscribed to the Google Groups >>> "Castle Project Users" group. >>> To post to this group, send email to >>> [email protected]. >>> To unsubscribe from this group, send email to >>> [email protected]<castle-project-users%[email protected]> >>> . >>> For more options, visit this group at >>> http://groups.google.com/group/castle-project-users?hl=en. >>> >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Castle Project Users" group. >> To post to this group, send email to >> [email protected]. >> To unsubscribe from this group, send email to >> [email protected]. >> For more options, visit this group at >> http://groups.google.com/group/castle-project-users?hl=en. >> >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Castle Project Users" group. >> To post to this group, send email to >> [email protected]. >> To unsubscribe from this group, send email to >> [email protected]<castle-project-users%[email protected]> >> . >> For more options, visit this group at >> http://groups.google.com/group/castle-project-users?hl=en. >> > > -- > You received this message because you are subscribed to the Google Groups > "Castle Project Users" group. > To post to this group, send email to [email protected] > . > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/castle-project-users?hl=en. > > > -- > You received this message because you are subscribed to the Google Groups > "Castle Project Users" group. > To post to this group, send email to [email protected] > . > To unsubscribe from this group, send email to > [email protected]<castle-project-users%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/castle-project-users?hl=en. > -- You received this message because you are subscribed to the Google Groups "Castle Project Users" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/castle-project-users?hl=en.
