[M3devel] small design problem in ButtonVBT?

[hendrik at topoi.pooq.com]

On Thu, Aug 27, 2009 at 11:53:27PM -0400, Tony Hosking wrote:
> Right.  Plus, overriding copies the entire method suite for the new  
> type.

Now that makes sense.  I'll re-evaluate my suggestion in this light.

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

It's more a matter of letting the action procedure or method have clean 
access to application data in the subclassed function.  But your code 
suggests a simpler way to do this than either your code or mine.  It 
takes a run-time type check, which is conceptually messy.  But the code 
is smaller, and seems to have a smaller memory footprint.

See below.

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

Yes.  That could be an issue.  But only if act is also overridden, which 
won't happen in legacy code, since the defauly act behaves identically 
to the action procedure.

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

But I can simplify this further.

TYPE T = ButtonVBT.T OBJECT (* as you propose. *)

For each action that I had wanted a subclass for, write

PROCEDURE Action(vv: T; cd: VBT.MouseRec) = ...
VAR v : T;
BEGIN
v := NARROW(vv, T);
... normal code for performing the action
END Action;

and initialize my T thingy using this Action.

So instead of multiple subclasses to get the various actions, I get two 
extra lines of code in each action procedure.  And I don't have to use 
GetProp or PutProp (which probably involve some kind of search).  The 
run-time cost is the type-check in the NARROW(vv, T), which is probably 
cheaper than a method-dispatch on act.

Hmmm.  I'm starting to think you're right, and my change is unnecessary.

I'll have a look in my applicatin code, and see if there are further 
glitches.  I'm not expecting any.

-- hendrik

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