[M3devel] small design problem in ButtonVBT?

[Tony Hosking]

Right.  Plus, overriding copies the entire method suite for the new  
type.

On 27 Aug 2009, at 23:28, Jay K wrote:

>
> I think I know the reason it is this way.
> It is very common to override action and nothing else.
> It is "inconvenient" to derive a new type, just to change one  
> function.
>
> - Jay
>
> ----------------------------------------
>> Date: Thu, 27 Aug 2009 23:17:23 -0400
>> From: hendrik at topoi.pooq.com
>> To: m3devel at elegosoft.com
>> Subject: Re: [M3devel] small design problem in ButtonVBT?
>>
>> On Thu, Aug 27, 2009 at 03:15:41PM -0400, Tony Hosking wrote:
>>> I am nervous about having the two alternative entries to the action
>>> procedure that other code might not be aware of. Yes, you've  
>>> modified
>>> the default mouse method to call the action method instead of the
>>> procedure, but what about other places where the mouse method has  
>>> been
>>> overridden.
>>>
>>> If you want to be smart about it in your own code you can simply
>>> invert your suggestion and do the following:
>>>
>>> TYPE T = ButtonVBT.T OBJECT
>>> METHODS action(cd: VBT.MouseRec) := Action;
>>>
>>> PROCEDURE Action(v: T; cd: VBT.MouseRec) = ...
>>>
>>> PROCEDURE ActionProc(v: ButtonVBT.T; cd: VBT.MouseRec) =
>>> BEGIN
>>> NARROW(v, T).action(cd);
>>> END ActionProc;
>>>
>>> and initialize your T thingy with ButtonVBT.New(..., action :=
>>> ActionProc; ...)
>>>
>>> Then in your code you can go ahead and override the action method to
>>> your hearts content.
>>
>> This kind of thing would work. But, like the existing semantics of
>> ButtonVBT, it seems to be fighting the language instead of working  
>> with
>> it. I'd be happiest to get rid of the action variable altogether, but
>> that would be a massively incompatible change, and would require  
>> change
>> to a lot of existing code. What I proposed is compatible with  
>> existing
>> code, and also has the advantage that someone who subclasses  
>> ButtonVBT.T
>> and overrides act can completely ignore the action variable  
>> altogether.
>>
>> You say
>>
>>> and initialize your T thingy with ButtonVBT.New(..., action :=
>>> ActionProc; ...)
>>
>> But that requires every object I make of type T or any sibclass to be
>> explicitly initialised with ActionProc, certainly a nuisance, and
>> certainly error-prone.
>>
>> The inelegance I see with my approach is that there are two  
>> mechanisms
>> for specifying the action to be taken. Either will work without the
>> programmer paying any attention to the other, but if you use both,  
>> one
>> will override the other. This is not a problem with legacy software.
>> I'll have to come up with some documentation that makes this clear.
>> It's the inelegance that comes from backward compatibility, and I  
>> think
>> my solution is less inelegant, and easier to use, than yours. Yours,
>> however, involves no change in existing code.
>>
>>>
>>> Am I just being paranoid?
>>
>> No. Perhaps just paranoid about different things.
>>
>> I had hoped for someone to come up with a solid technical reason  
>> for the
>> action variable. The original designers were clever people, and I'd
>> *still* like to know why they thought it was a good idea. It's  
>> awkward
>> and error-prone to use, and they had to use the PutProp/GetProp  
>> kludge
>> just to approximate what the language provided for free with  
>> inheritance.
>>
>> I'm tempted to go over the entire code-base and change the buttons to
>> use act instead of action, but I have to admit I'm not tempted  
>> enough to
>> actually do it...
>>
>> -- hendrik
>>
>>> On 27 Aug 2009, at 08:17, Olaf Wagner wrote:
>>>
>>>> Though I promised to do it, I have lost sight of this task while
>>>> trying to build some RC3 archives on Windows. As I'm not at home  
>>>> this
>>>> evening, can somebody else please commit Hendrik's extension?
>>>>
>>>> Is this something we should should have in the release branch, too?
>>>> Probably not necessary?
>>>>
>>>> Olaf
>>>>
>>>> Quoting hendrik at topoi.pooq.com:
>>>>
>>>>> On Wed, Apr 01, 2009 at 12:03:47PM +0200, Olaf Wagner wrote:
>>>>>> Quoting hendrik at topoi.pooq.com:
>>>>>>
>>>>>>> The BottonVBT contains an action, which is a procedure rather
>>>>>> than a
>>>>>>> method. b This makes it awkward to subclass ButtonVBT becaue the
>>>>>> action
>>>>>>> will still expect its first parameter to be a ButtonVBT  
>>>>>>> instead of
>>>>>>> belonging to the subclass, and an explicit downcast of some sort
>>>>>> will be
>>>>>>> needed to acess the subclass's members.
>>>>>>>
>>>>>>> The Trestle manual says this was a deliberate decision:
>>>>>>>
>>>>>>> : The action procedure is a field rather than a method in order
>>>>>> to allow
>>>>>>> : buttons with different action procedures to share their method
>>>>>> suites.
>>>>>>>
>>>>>>> I, however, find it massively inconvenient because it mans I  
>>>>>>> have
>>>>>> to
>>>>>>> resort to downcasting or the very kludgey VBT.GetProp to access
>>>>>> what
>>>>>>> should be just there. I'd rather the action procedure was a
>>>>>> method.
>>>>>>>
>>>>>>> But it should be possible to have the best of both worlds.  
>>>>>>> Have an
>>>>>>> action2 (better name wanted here) method that is available for
>>>>>>> overriding, and by default calls the procedure in the existing
>>>>>> action.
>>>>>>> field. That action2 method wounl then be called by Trestle
>>>>>> wherever
>>>>>>> action is now called.
>>>>>>>
>>>>>>> Does anyone see problems with this?
>>>>>>
>>>>>> Sounds fine to me offhand. You should test your extensions with
>>>>>> some of the existing larger applications, like mentor and Juno-2,
>>>>>> of course.
>>>>>
>>>>> OK. Here's the patch. Would it be possible for someone to check it
>>>>> in
>>>>> for me?
>>>>>
>>>>> mentor runs fine with this change; I watched it animate a heap- 
>>>>> sort.
>>>>>
>>>>> I have no idea what Juno is supposed to do, but it did respond to
>>>>> button-pushes.
>>>>>
>>>>>
>>>>> hendrik at lovesong:~/cm3/love-sq/cm3/m3-ui/ui/src/split$ cvs diff
>>>>> cvs diff: Diffing .
>>>>> Index: ButtonVBT.i3
>>>>> = 
>>>>> ==================================================================
>>>>> RCS file: /usr/cvs/cm3/m3-ui/ui/src/split/ButtonVBT.i3,v
>>>>> retrieving revision 1.2
>>>>> diff -r1.2 ButtonVBT.i3
>>>>> 39a40
>>>>>> act(READONLY cd: VBT.MouseRec);
>>>>> Index: ButtonVBT.m3
>>>>> = 
>>>>> ==================================================================
>>>>> RCS file: /usr/cvs/cm3/m3-ui/ui/src/split/ButtonVBT.m3,v
>>>>> retrieving revision 1.2
>>>>> diff -r1.2 ButtonVBT.m3
>>>>> 28c28,29
>>>>> < init := Be
>>>>> ---
>>>>>> init := Be;
>>>>>> act := act
>>>>> 46a48,52
>>>>>> PROCEDURE act(v : T; READONLY cd: VBT.MouseRec) =
>>>>>> BEGIN
>>>>>> v.action(v, cd);
>>>>>> END act;
>>>>>>
>>>>> 59c65
>>>>> < v.action(v, cd);
>>>>> ---
>>>>>> v.act(cd);
>>>>> hendrik at lovesong:~/cm3/love-sq/cm3/m3-ui/ui/src/split$
>>>>>
>>>>> -- hendrik
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Olaf Wagner -- elego Software Solutions GmbH
>>>> Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin,
>>>> Germany
>>>> phone: +49 30 23 45 86 96 mobile: +49 177 2345 869 fax: +49 30 23
>>>> 45 86 95
>>>> http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz:
>>>> Berlin
>>>> Handelregister: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr:
>>>> DE163214194
>>>>
>>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://m3lists.elegosoft.com/pipermail/m3devel/attachments/20090827/119fc226/attachment-0002.html>