On Mon, Sep 13, 2010 at 1:38 AM, Gavin Panella <[email protected]> wrote: > On 11 September 2010 22:04, Robert Collins <[email protected]> > wrote: > ... >> This seems to draw a fairly large bow : adaption not working in one >> context -> don't use adaption anywhere. > > Yeah, this is my main concern.
I'd suspect the tests failed because there are tests using a layer without the zca present, that call code which previously didn't actually /need/ any adapters etc. That would mean it ran without the zcml parsed etc etc. Thats not consistent with 'only fails in a full run' though. adding debug info is probably how I'd start tackling the problem. >> I think the declarative adaption facilities of the ZCA are great; but >> that adaption isn't a panacea : the boilerplate you described would be >> better written with delegation, not adaption. > > Fair. > > The example was taken from StructuralSubscriptionTargetMixin, and > delegation would work well there. The classes that inherit this mixin, > Product for example, are fairly big already so my thinking was along > the lines of: > > - I wouldn't want to add yet more responsibility to these classes, > > - Being able to say "give me an object that lets me query and modify > structural subscriptions for this thing" sounds good, > > - Adaption seems like a good way to do that, and relieves the already > large classes from all responsibility, > > I might be wielding a hammer and seeing nails. Would you consider > adapters for this scenario? So, to paraphrase (checking I have it right): - there is common code to deal with things that can be structurally subscribed to - there is some unique code per-type to deal with the same stuff Without adaption I would probably have tackled it something like: - things that can be structurally subscribed to have a factory to give you their structural subscription object - that object is tailored to any uniqueness needed - I would not have used a MixIn With adaption, I think I would: - have an adapter to get IStructuralSubscriptionTarget from things that can be subscribed to - the resulting IStructuralSubscriptionTarget is tailored to any uniqueness needed So, if the current situation is that ISST adaption is done by using a mixin on e.g. Product, then I think that you should either: - customise the mixin per target type (mixin StructuralSubscriptionTargetProduct to Product, StructuralSubscriptionTargetDistribution to Distribution) - the uniqueness will be in the mixin - stop using a mixin and actually use an adapter :) What I definitely wouldn't do is half and half, it seems likely to neither add clarity, nor clear things up. My personal bias is something like: - use dependency injection techniques when doing dependency injection - use simpler code otherwise But thats handwavy and not entirely true ;P. >> # Note that writing it this way makes it reasonably clear that we >> # aren't using delegation/composition as much as we could in this area. >> self.populateArgs() >> >> Similarly, I'm very unconvinced that using adaption for the property >> cache was useful or helpful. I like the new API, but it seems like it >> could be more pithy, more reusable and faster if it was just plain >> python. > > I was tempted by the ZCA! I honestly thought we might need the > flexibility that could bring, but turns out we didn't. So for the property cache the question is : will we want a different implementation of the cache while testing/deploying. I think the answer is 'No', so while adaption is prettty quick, its still almost certainly slower than just deferencing the object to get the cache. > So the best way to fix bug 628762 is probably to do away with > adaption, but I'd like to repair my confidence in Zope too (or repair > my knowledge). I'm happy either way: you've stepped up and made the cached property API much cleaner; whether it uses ZCA is a relatively minor detail. If there is a zerious issue in the ZCA, knowing and fixing it is important! -Rob _______________________________________________ Mailing list: https://launchpad.net/~launchpad-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-dev More help : https://help.launchpad.net/ListHelp
