[M3devel] small design problem in ButtonVBT?

[Jay K]

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