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

Gary Poster gary.poster at gmail.com
Tue Sep 8 22:12:14 EDT 2009


On Sep 8, 2009, at 11:30 AM, Martijn Faassen wrote:

> Hey,

Hi Martijn.  Thank you for the thoughtful reply.

>
> 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'.

I disagree. :-)

In the context of zcml, "provides" comes within an "adapter" tag.  In  
the context of a single function call, "provideAdapter" clearly says  
what you are doing (providing an adapter).  Instead, in the example I  
gave above, the verb "provides" is in the same context as  
"implements", and there's nothing to clarify that we mean something  
else here.  I don't think the examples you give contradict my point.

The email would have not been written, or at least looked very  
different, if the grokcore call had been "providesAdapter" (or  
"provides_adapter").  Readability trumps brevity in my book, and  
"provides" is confusing.

>> 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.

Sure, as with the standard Zope behavior.

>> 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.

Well, I don't buy that concern myself, but the "there's too many ways  
to do it" point you bring up below does ring true to me.

>
>> 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..

Perhaps so.  I do find your argument here very compelling.

>> 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.

Except that this was in the context of me changing the behavior; but  
no matter, I'm retracting that.  Your "too many spellings" convinced  
me of that one.

> The whole thing is called *providing* in zope.component. The
> term is sadly overloaded,

I honestly think that the overloading begins to become a problem (or  
at least much more of a problem) with the grokcore spelling that I am  
concerned about; in the Zope usages, I think the contexts I mentioned  
above make the readability much clearer.

> 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.

Yeah, I considered and rejected "register" as being too meaningless,  
as I suspect you all did.

> (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.

Yes, as I said, you convinced me that my suggestion was not a win.   
However, I'm also more convinced that the name of  
"grokcore.component.provides" is a problem.  My current preference is  
for "provides_adapter".  Reading this, I understand what the grok  
directive is doing.

class FeaturedCookbookLink(CookbookTopLevelObject):
     """A link to the currently featured cookbook."""
     zope.interface.implements(ICookbookObject, ITopLevelEntryLink)
     grokcore.component.provides_adapter(ITopLevelEntryLink)

If you do a "from foo import bar" style (still advocated in some  
quarters, and used at Canonical) then the distinction is even more  
stark, in my opinion.  Current:

class FeaturedCookbookLink(CookbookTopLevelObject):
     """A link to the currently featured cookbook."""
    implements(ICookbookObject, ITopLevelEntryLink)
     provides(ITopLevelEntryLink)

My read: huh?

Proposed:

class FeaturedCookbookLink(CookbookTopLevelObject):
     """A link to the currently featured cookbook."""
    implements(ICookbookObject, ITopLevelEntryLink)
    provides_adapter(ITopLevelEntryLink)

My read: ah so!

Thanks again,

Gary


More information about the Grok-dev mailing list