[Product-Developers] Re: collective.monkeypatcher -- awkward names

Jean Jordaan jean.jordaan at gmail.com
Thu Feb 11 04:26:51 UTC 2010


Hi Martin & all

Current:

>>  <monkey:patch
>>     description="Add XML-RPC capability to SomeModule"
>>     module="Products.SomeProduct.SomeModule"
>>     original="readRemoteProject"
>>     ignoreOriginal="True"
>>     replacement=".SomeModule.readRemoteProject"
>>     />

Suggested:

>>  <monkey:patch
>>     description="Add XML-RPC capability to SomeModule"
>>     module="Products.SomeProduct.SomeModule"
>>     checkOriginal="False"
>>     patch=".SomeModule.readRemoteProject"
>>     />
>
> +1
>
> I'm not sure what ignoreOriginal is doing in this case; I'd probably just
> ignore it.

In the original, 'ignoreOriginal="True"' is required because the
original doesn't exist. I'm adding a new function. If you ignore it,
you'll get an exception.

In my suggested version, you have to switch off the existence check that
'patch' normally does, because in this case you know that there is no
original. That said, I'd like to suggest 'new' instead of
'checkOriginal'::

  <monkey:patch
     description="Add XML-RPC capability to SomeModule"
     module="Products.SomeProduct.SomeModule"
     new="True"
     patch=".SomeModule.readRemoteProject"
     />

I.e. the normal semantics of 'patch' is to *replace* an *existing*
member (function/method/property/...). If that member doesn't exist, an
exception is raised. If 'new="True"', it expects to add a new member. If
it finds the name already taken, it raises an exception
('ignoreOriginal' and 'checkOriginal' both only check for presence, not
for absence).

>> If I simply want to replace an existing function of the same name,
>> checking for the original::
>>
>>  <monkey:patch
>>     description="Add XML-RPC capability to SomeModule"
>>     module="Products.SomeProduct.SomeModule"
>>     patch=".SomeModule.readRemoteProject"
>>     />
>
> -1
>
> I think you should have to explicitly say what you're patching. Just
> looking for something with the same name is too magical. The whole
> point of collective.monkeypatcher is to make everything very explicit.

It is explicit; there's no magic. The documentation of 'patch' will say
that it expects to find a member of the same name. If that member
doesn't exist, monkeypatcher will raise an exception. That will
encourage keeping the names of the patch and the original the same,
which will make the patching product more readable and amenable to e.g.
ctags, because the names from the original API are preserved.

>> If I'm replacing an existing function, and for some reason the
>> 'description' attribute is insufficient to describe what I'm doing and I
>> want to name my method something different::
>>
>>  <monkey:patch
>>     description="Add XML-RPC capability to SomeModule"
>>     module="Products.SomeProduct.SomeModule"
>>     original="readRemoteProject"
>>     patch=".SomeModule.readRemoteProject_myownspecialweirdname"
>>     />
>
> This is the current basic syntax, no? I think this is the appropriate way
> still for both this and the case above where the patch function and the
> original function happened to have the same name.

Yes, this is like the current. I think the repeat is makework and a
chance for typos though. I hope you agree that documentation like "by
default, 'patch' replaces a member of the same name on the patched
object" makes things clear enough.

Of course, I may be completely missing something that invalidates my
proposal .. please tell me if I am!

-- 
jean                                              . .. .... //\\\oo///\\




More information about the Product-Developers mailing list