[Grok-dev] concerns about grokcore.component.provides

Martijn Faassen faassen at startifact.com
Tue Sep 8 11:30:30 EDT 2009


Hey,

Gary Poster wrote:
> The Launchpad team successfully made its first foray into grok land  
> today by switching some of lazr.restful to use it.  It has turned out  
> nicely so far, and the next step is to make some custom martian  
> declarations.  Thank you!

Cool!

> In the course of this, though, we came to a couple of concerns about  
> grokcore.component.provides that I'd like to share.  I understand why  
> the decisions have been made the way they are, but I'd at least like  
> to share our thoughts.
> 
> Consider this only slightly contrived snippet, which is an example of  
> what you find repeatedly in our code with the new change:
> 
> class FeaturedCookbookLink(CookbookTopLevelObject):
>      """A link to the currently featured cookbook."""
>      zope.interface.implements(ICookbookObject, ITopLevelEntryLink)
>      grokcore.component.provides(ITopLevelEntryLink)
> 
> First, as an old Zope 3 hand, I personally find the name  
> grokcore.component.provides very confusing.  For the zope.interface  
> package, of course, "provides" describes what an instance directly  
> offers, while "implements" describes the special case of what a class  
> produces.  I suspect the word "provides" was chosen for the  
> grokcore.component package because it mirrors the zcml registration of  
> an adapter.  However, in context, I find it very confusing, especially  
> juxtaposed with the "implements" call.

We had some discussions in the past about what to call this. Do you have 
a suggestion for a better name? We've followed not just the ZCML 
registration, but also the lower-level 'provideAdapter' registration 
that is used. That's at least a strong argument in favor of 'provides'.

> Second, this looks an awful lot like a DRY "violation"--one of the  
> things I thought grok was particularly trying to avoid. 

Actually if there is only *one* interface implemented, the provides 
directive falls back on that interface (you don't have to add provides 
at all). It only is needed if there are more interfaces to choose from.

> Shouldn't you  
> be able to make a single spelling for the class, simultaneously  
> declaring that the interface is implemented and that it should be  
> registered as such?  In other words, shouldn't this be equivalent to  
> the above?
> 
> class FeaturedCookbookLink(CookbookTopLevelObject):
>      """A link to the currently featured cookbook."""
>      zope.interface.implements(ICookbookObject)
>      grokcore.component.provides(ITopLevelEntryLink)

That's a different way to bring about DRY. I think this is also a bit 
worrying, as normally you just have to look at implements() to see what 
interfaces the object will provide. One now would need to look at two 
places instead of one.

> I can grudgingly acknowledge an edge case of wanting to be able to  
> register an object for an interface that it does not implement, but  
> what an edge case!

Probably not very common, indeed. I'm not worried about this edge case much.

[snip edge case]

> My main goal is to see if I can make someone--say, eventually,  
> Martijn ;-)--agree with us, and change things a bit.  However, I feel  
> like I'm whining if I complain without trying to offer a solution, so  
> I'll try to make a couple.  I'd be thrilled if someone else agreed  
> with our concerns, but came up with a better solution.
> 
> Option 1: you ignore my concern about naming, and just address the DRY  
> violation.  grokcore.component.provides will automatically add the  
> interface to the class, so my second, theoretical example does in fact  
> produce the same thing as the first.  Perhaps you add a flag or  
> another function that has the current behavior of not adding the  
> "implements" declaration.

Currently we have the following cases:

class Adapter(grok.Adapter):
     grok.implements(IFoo)
     grok.provides(IFoo)

which is equivalent to this:

class Adapter(grok.Adapter):
     grok.implements(IFoo)

and we also have this case:

class Adapter(grok.Adapter):
     grok.implements(IFoo, IBar)
     grok.provides(IBar)

In this case grok.provides is mandatory as there is no way to know which 
interface this adapter needs to be registered for.

If we add your proposal so that grok.provides actually also causes the 
adapter to implement the adapter, we'd have the following possibilities:

class Adapter(grok.Adapter):
     grok.implements(IFoo)
     grok.provides(IFoo)

which is equivalent to this:

class Adapter(grok.Adapter):
     grok.implements(IFoo)

which is equivalent to this:

class Adapter(grok.Adapter):
     grok.provides(IFoo)

and we have these cases:

class Adapter(grok.Adapter):
     grok.implements(IFoo, IBar)
     grok.provides(IBar)

which is equivalent to this:

class Adapter(grok.Adapter):
     grok.implements(IFoo)
     grok.provides(IBar)

I'm a bit worried about the amount of equivalent cases we are generating 
here. Does this become easier or harder to reason about? Currently 
grok.provides is strictly about registration, but with your proposal 
it'd become almost (but not quite!) a synonym for 'implements'. 
Reasoning about the rules might be more complicated..

> Option 2: "grokcore.component.provides" is deprecated in favor of  
> "grokcore.component.instancesProvide".  The new declaration has the  
> behavior I describe in option 1.  It would result in this:
> 
> class FeaturedCookbookLink(CookbookTopLevelObject):
>      """A link to the currently featured cookbook."""
>      zope.interface.implements(ICookbookObject)
>      grokcore.component.instancesProvide(ITopLevelEntryLink)

but currently what you propose you call 'instancesProvide' is really 
only needed to fill in the provides argument to 'provideAdapter' or 
'provideUtility'. It's not really about what the instances provide, it's 
about how this thing is going to be registered into the component 
registry. The whole thing is called *providing* in zope.component. The 
term is sadly overloaded, but I'm not sure that calling it 
'instancesProvide' will make this any clearer.

I think previous discussions also mentioned the word 'register' in this 
context. I doesn't quit work as nicely as 'implements' and 'provides' as 
it'd be an active verb for a directive name, but you could have 
grok.register(ITopEntryLink). Anyway, just some terminology to think about.

(besides this, I think commonly used directives should be one word if at 
all possible, and multi-word directives I think we'd use underscores for 
(instances_provide)

> Thanks again for grokcore/martian.  They have given us a nice win, and  
> we're interested in looking at them more.

I'm happy to hear about that. To be clear, even though I'm pushing back, 
I'm very happy to get some feedback and I hope others will join in this 
discussion. I hope there is a way to improve upon the 'provides' 
behavior there is now, but I'm not convinced your proposals are a net 
win. But perhaps we can get there.

Regards,

Martijn



More information about the Grok-dev mailing list