Home » Modeling » EMF » Composing Switches
|
| Re: Composing Switches [message #479303 is a reply to message #479291] |
Mon, 10 August 2009 15:51   |
 Ed Merks
Messages: 33140
Registered: July 2009 |
Senior Member |
|
|
Adrian,
That's an interesting idea... I wonder though... In the end, switches
need to do something specific and useful yet the generated XyzSwitch is
effectively useless as is, i.e., it is only useful when one derives from
it to do specific things with one or more of the case methods. As such,
when building a specific derived switch that does particular things and
hence make sense to compose with other derived switches that do that
same specific thing, a common interface that "declares" the interesting
thing they do could be introduced at that point...j
Note too that your example is incomplete. I.e., ComposedSwitch doesn't
implement the getEPackage method, nor can it...
Adrian Price wrote:
> Good day EMF NG,
>
> A quick suggestion: wouldn't it be useful for the EMF-generated switch
> classes to be more readily composable? Something like:
>
> public interface org.eclipse.emf.ecore.util.Switch<T> {
> EPackage getEPackage();
> <T> doSwitch(EObject theEObject);
> }
>
> public class XxxSwitch<T> implements Switch<T> {
> // Note: XxxSwitchImpl.getEPackage() { return XxxPackage.eINSTANCE; }
> ...
> }
>
> public class YyySwitch<T> implements Switch<T> {
> // Note: YyySwitchImpl.getEPackage() { return YyyPackage.eINSTANCE; }
> ...
> }
>
> /**
> * Example implementation; others possible.
> */
> public class ComposedSwitch<T> implements Switch<T> {
> private final Map<EPackage, Switch<T>> delegates = new HashMap<EPackage,
> Switch<T>>();
>
> public ComposedSwitch<T>(Switch<T>... delegates) {
> for (Switch<T> delegate : delegates)
> this.delegates.put(delegate.getEPackage(), delegate);
> }
>
> public T doSwitch(EObject theEObject) {
> return
> delegates.get(theEObject.eClass().getEPackage()).doSwitch(th eEObject);
> }
> }
>
> Thanks,
>
> Adrian Price
> Senior Architect | TIBCO Software, Inc.
>
>
>
Ed Merks
Professional Support: https://www.macromodeling.com/
|
|
|
| Re: Composing Switches [message #479316 is a reply to message #479303] |
Mon, 10 August 2009 16:45 |
Adrian Price
Messages: 61
Registered: July 2009 |
Member |
|
|
Hi Ed,
Thanks for the quick response. Darn, you're right about the silly logical
flaw in my composed switch - now that you point it out I recall hitting a
similar problem a couple of months back when I tried out something similar
in code. Guess I'd have noticed it immediately if I'd have written this
example in a Java editor instead of an e-mail client! I suppose the composed
switch wouldn't necessarily have to implement Switch<T> although it would be
somewhat inelegant for it not to do so.
I have various places (factories mostly) in an application that uses several
different metamodels and where I need to compose switch subclasses -
hitherto each composed switch has been individually crafted for the packages
it handles - I was just pondering ways of doing something more geneneric. I
take your point about the uselessness of the generated XxxSwitch base
classes and I suppose it would also be possible to have the MyXxxSwitch
classes also implement an interface with a getEPackage() method on it. Then
the Switch<T> interface itself becomes less of a necessity but still
potentially useful (minus the getEPackage() method of course!).
Anyhow, just a thought.
Thanks,
Adrian
"Ed Merks" <Ed.Merks@gmail.com> wrote in message
news:h5pfmv$bg3$1@build.eclipse.org...
> Adrian,
>
> That's an interesting idea... I wonder though... In the end, switches
> need to do something specific and useful yet the generated XyzSwitch is
> effectively useless as is, i.e., it is only useful when one derives from
> it to do specific things with one or more of the case methods. As such,
> when building a specific derived switch that does particular things and
> hence make sense to compose with other derived switches that do that same
> specific thing, a common interface that "declares" the interesting thing
> they do could be introduced at that point...j
>
> Note too that your example is incomplete. I.e., ComposedSwitch doesn't
> implement the getEPackage method, nor can it...
>
>
> Adrian Price wrote:
>> Good day EMF NG,
>>
>> A quick suggestion: wouldn't it be useful for the EMF-generated switch
>> classes to be more readily composable? Something like:
>>
>> public interface org.eclipse.emf.ecore.util.Switch<T> {
>> EPackage getEPackage();
>> <T> doSwitch(EObject theEObject);
>> }
>>
>> public class XxxSwitch<T> implements Switch<T> {
>> // Note: XxxSwitchImpl.getEPackage() { return XxxPackage.eINSTANCE; }
>> ...
>> }
>>
>> public class YyySwitch<T> implements Switch<T> {
>> // Note: YyySwitchImpl.getEPackage() { return YyyPackage.eINSTANCE; }
>> ...
>> }
>>
>> /**
>> * Example implementation; others possible.
>> */
>> public class ComposedSwitch<T> implements Switch<T> {
>> private final Map<EPackage, Switch<T>> delegates = new
>> HashMap<EPackage, Switch<T>>();
>>
>> public ComposedSwitch<T>(Switch<T>... delegates) {
>> for (Switch<T> delegate : delegates)
>> this.delegates.put(delegate.getEPackage(), delegate);
>> }
>>
>> public T doSwitch(EObject theEObject) {
>> return
>> delegates.get(theEObject.eClass().getEPackage()).doSwitch(th eEObject);
>> }
>> }
>>
>> Thanks,
>>
>> Adrian Price
>> Senior Architect | TIBCO Software, Inc.
>>
>>
|
|
|
| Re: Composing Switches [message #479880 is a reply to message #479303] |
Wed, 12 August 2009 21:30 |
Adrian Price
Messages: 61
Registered: July 2009 |
Member |
|
|
Hi Ed,
I certainly take your point that one could hand-crank all the subclasses to
support composition, but wouldn't it be nice if genned base switch classes
did it for free? Here's a revised, cost-free approach that could actually
work:
public interface org.eclipse.emf.ecore.util.Switch<T> {
T doSwitch(EObject theEObject);
boolean supportsEPackage(EPackage pkg);
}
public class XxxSwitch<T> implements Switch<T> {
public boolean supportsEPackage(EPackage pkg) {
return pkg == XxxPackage.eINSTANCE;
}
...
}
/** Example implementation; others possible. */
public class ComposedSwitch<T> implements Switch<T> {
private final Switch<T> cacheMiss = new Switch<T>() {
public T doSwitch(EObject theEObject) { return null; }
public boolean supportsEPackage(EPackage pkg) { return false; }
};
private final Switch<T>[] delegates;
private final Map<EPackage, Switch<T>> delegatesByPackage = new
HashMap<EPackage, Switch<T>>();
public ComposedSwitch(Switch<T>... delegates) {
this.delegates = delegates.clone();
}
public T doSwitch(EObject theEObject) {
Switch<T> delegate =
findDelegate(theEObject.eClass().getEPackage());
return delegate == null ? null : delegate.doSwitch(theEObject);
}
private Switch<T> findDelegate(EPackage pkg) {
Switch<T> delegate = delegatesByPackage.get(pkg);
if (delegate == null) {
delegate = cacheMiss;
for (Switch<T> sw : delegates) {
if (sw.supportsEPackage(pkg)) {
delegate = sw;
break;
}
}
delegatesByPackage.put(pkg, delegate);
}
return delegate == cacheMiss ? null : delegate;
}
public boolean supportsEPackage(EPackage pkg) {
return findDelegate(pkg) != null;
}
}
Cheers,
Adrian Price
Senior Architect | TIBCO Software, Inc.
"Ed Merks" <Ed.Merks@gmail.com> wrote in message
news:h5pfmv$bg3$1@build.eclipse.org...
> Adrian,
>
> That's an interesting idea... I wonder though... In the end, switches
> need to do something specific and useful yet the generated XyzSwitch is
> effectively useless as is, i.e., it is only useful when one derives from
> it to do specific things with one or more of the case methods. As such,
> when building a specific derived switch that does particular things and
> hence make sense to compose with other derived switches that do that same
> specific thing, a common interface that "declares" the interesting thing
> they do could be introduced at that point...j
<snip/>
|
|
|
| Re: Composing Switches [message #479928 is a reply to message #479880] |
Thu, 13 August 2009 07:58 |
Ed Merks
Messages: 33140
Registered: July 2009 |
Senior Member |
|
|
This is a multi-part message in MIME format.
--------------040307050607080002080705
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Adrian,
Comments below.
Adrian Price wrote:
> Hi Ed,
>
> I certainly take your point that one could hand-crank all the subclasses to
> support composition, but wouldn't it be nice if genned base switch classes
> did it for free?
Sounds like more work for me though. :-P
> Here's a revised, cost-free
Cost to whom? :-P
> approach that could actually
> work:
>
> public interface org.eclipse.emf.ecore.util.Switch<T> {
>
New interfaces aren't entirely free.
> T doSwitch(EObject theEObject);
> boolean supportsEPackage(EPackage pkg);
>
Or perhaps isSwitchFor...
> }
>
> public class XxxSwitch<T> implements Switch<T> {
>
One would have to rely on models being regenerated to support this.
That's not a problem, except that many projects are lazy about, or worse
yet averse to, regenerating. Oh well, too bad for them.
> public boolean supportsEPackage(EPackage pkg) {
> return pkg == XxxPackage.eINSTANCE;
> }
> ...
> }
>
> /** Example implementation; others possible. */
>
It would be good to provide a high quality reusable base.
> public class ComposedSwitch<T> implements Switch<T> {
> private final Switch<T> cacheMiss = new Switch<T>() {
> public T doSwitch(EObject theEObject) { return null; }
> public boolean supportsEPackage(EPackage pkg) { return false; }
> };
> private final Switch<T>[] delegates;
> private final Map<EPackage, Switch<T>> delegatesByPackage = new
> HashMap<EPackage, Switch<T>>();
>
> public ComposedSwitch(Switch<T>... delegates) {
> this.delegates = delegates.clone();
> }
>
> public T doSwitch(EObject theEObject) {
> Switch<T> delegate =
> findDelegate(theEObject.eClass().getEPackage());
> return delegate == null ? null : delegate.doSwitch(theEObject);
>
Would it be better for the composed switch to support the same type of
logic as the generated switch?
protected T doSwitch(EClass theEClass, EObject theEObject)
{
if (theEClass.eContainer() == modelPackage)
{
return doSwitch(theEClass.getClassifierID(), theEObject);
}
else
{
List<EClass> eSuperTypes = theEClass.getESuperTypes();
return
eSuperTypes.isEmpty() ?
defaultCase(theEObject) :
doSwitch(eSuperTypes.get(0), theEObject);
}
}
I suppose the == modelPackage should call the new tester method and then
this entire method could be moved to a base class; that would actually
save some byte code which might well offset the additional byte code.
It makes me wonder if an abstract base class shared by generated
switches and the composed switch instead of a interface might not be
just as functional...
> }
>
> private Switch<T> findDelegate(EPackage pkg) {
> Switch<T> delegate = delegatesByPackage.get(pkg);
> if (delegate == null) {
> delegate = cacheMiss;
>
I'm not sure I understand the value of cacheMiss given it's converted
back into null...
> for (Switch<T> sw : delegates) {
> if (sw.supportsEPackage(pkg)) {
> delegate = sw;
> break;
> }
> }
> delegatesByPackage.put(pkg, delegate);
>
Would it be better for the instance to be thread safe?
> }
> return delegate == cacheMiss ? null : delegate;
> }
>
> public boolean supportsEPackage(EPackage pkg) {
> return findDelegate(pkg) != null;
> }
> }
>
>
It all sounds like quite a bit of work, for me. :-P Oh well, too bad
for me...
> Cheers,
>
> Adrian Price
> Senior Architect | TIBCO Software, Inc.
>
> "Ed Merks" <Ed.Merks@gmail.com> wrote in message
> news:h5pfmv$bg3$1@build.eclipse.org...
>
>> Adrian,
>>
>> That's an interesting idea... I wonder though... In the end, switches
>> need to do something specific and useful yet the generated XyzSwitch is
>> effectively useless as is, i.e., it is only useful when one derives from
>> it to do specific things with one or more of the case methods. As such,
>> when building a specific derived switch that does particular things and
>> hence make sense to compose with other derived switches that do that same
>> specific thing, a common interface that "declares" the interesting thing
>> they do could be introduced at that point...j
>>
>
> <snip/>
>
>
>
--------------040307050607080002080705
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
<title></title>
</head>
<body bgcolor="#ffffff" text="#000000">
Adrian,<br>
<br>
Comments below.<br>
<br>
Adrian Price wrote:
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap="">Hi Ed,
I certainly take your point that one could hand-crank all the subclasses to
support composition, but wouldn't it be nice if genned base switch classes
did it for free? </pre>
</blockquote>
Sounds like more work for me though. :-P<br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap="">Here's a revised, cost-free</pre>
</blockquote>
Cost to whom? :-P<br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> approach that could actually
work:
public interface org.eclipse.emf.ecore.util.Switch<T> {
</pre>
</blockquote>
New interfaces aren't entirely free.<br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> T doSwitch(EObject theEObject);
boolean supportsEPackage(EPackage pkg);
</pre>
</blockquote>
Or perhaps isSwitchFor...<br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap="">}
public class XxxSwitch<T> implements Switch<T> {
</pre>
</blockquote>
One would have to rely on models being regenerated to support this.
That's not a problem, except that many projects are lazy about, or
worse yet averse to, regenerating. Oh well, too bad for them.<br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> public boolean supportsEPackage(EPackage pkg) {
return pkg == XxxPackage.eINSTANCE;
}
...
}
/** Example implementation; others possible. */
</pre>
</blockquote>
It would be good to provide a high quality reusable base.<br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap="">public class ComposedSwitch<T> implements Switch<T> {
private final Switch<T> cacheMiss = new Switch<T>() {
public T doSwitch(EObject theEObject) { return null; }
public boolean supportsEPackage(EPackage pkg) { return false; }
};
private final Switch<T>[] delegates;
private final Map<EPackage, Switch<T>> delegatesByPackage = new
HashMap<EPackage, Switch<T>>();
public ComposedSwitch(Switch<T>... delegates) {
this.delegates = delegates.clone();
}
public T doSwitch(EObject theEObject) {
Switch<T> delegate =
findDelegate(theEObject.eClass().getEPackage());
return delegate == null ? null : delegate.doSwitch(theEObject);
</pre>
</blockquote>
Would it be better for the composed switch to support the same type of
logic as the generated switch?<br>
<blockquote> protected T doSwitch(EClass theEClass, EObject theEObject)<br>
{<br>
if (theEClass.eContainer() == modelPackage)<br>
{<br>
return doSwitch(theEClass.getClassifierID(), theEObject);<br>
}<br>
else<br>
{<br>
List<EClass> eSuperTypes = theEClass.getESuperTypes();<br>
return<br>
eSuperTypes.isEmpty() ?<br>
defaultCase(theEObject) :<br>
doSwitch(eSuperTypes.get(0), theEObject);<br>
}<br>
}<br>
</blockquote>
I suppose the == modelPackage should call the new tester method and
then this entire method could be moved to a base class; that would
actually save some byte code which might well offset the additional
byte code. It makes me wonder if an abstract base class shared by
generated switches and the composed switch instead of a interface might
not be just as functional... <br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> }
private Switch<T> findDelegate(EPackage pkg) {
Switch<T> delegate = delegatesByPackage.get(pkg);
if (delegate == null) {
delegate = cacheMiss;
</pre>
</blockquote>
I'm not sure I understand the value of cacheMiss given it's converted
back into null...<br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> for (Switch<T> sw : delegates) {
if (sw.supportsEPackage(pkg)) {
delegate = sw;
break;
}
}
delegatesByPackage.put(pkg, delegate);
</pre>
</blockquote>
Would it be better for the instance to be thread safe?<br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> }
return delegate == cacheMiss ? null : delegate;
}
public boolean supportsEPackage(EPackage pkg) {
return findDelegate(pkg) != null;
}
}
</pre>
</blockquote>
It all sounds like quite a bit of work, for me. :-P Oh well, too bad
for me...<br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap="">Cheers,
Adrian Price
Senior Architect | TIBCO Software, Inc.
"Ed Merks" <a class="moz-txt-link-rfc2396E" href="mailto:Ed.Merks@gmail.com"><Ed.Merks@gmail.com></a> wrote in message
<a class="moz-txt-link-freetext" href="news:h5pfmv$bg3$1@build.eclipse.org">news:h5pfmv$bg3$1@build.eclipse.org</a>...
</pre>
<blockquote type="cite">
<pre wrap="">Adrian,
That's an interesting idea... I wonder though... In the end, switches
need to do something specific and useful yet the generated XyzSwitch is
effectively useless as is, i.e., it is only useful when one derives from
it to do specific things with one or more of the case methods. As such,
when building a specific derived switch that does particular things and
hence make sense to compose with other derived switches that do that same
specific thing, a common interface that "declares" the interesting thing
they do could be introduced at that point...j
</pre>
</blockquote>
<pre wrap=""><!---->
<snip/>
</pre>
</blockquote>
</body>
</html>
--------------040307050607080002080705--
Ed Merks
Professional Support: https://www.macromodeling.com/
|
|
|
| Re: Composing Switches [message #479983 is a reply to message #479928] |
Thu, 13 August 2009 11:00 |
Adrian Price
Messages: 61
Registered: July 2009 |
Member |
|
|
This is a multi-part message in MIME format.
------=_NextPart_000_0049_01CA1C0D.A067A4E0
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Ed, thanks for your improvements, comments below.
Cheers,
--A
"Ed Merks" <Ed.Merks@gmail.com> wrote in message =
news:h60h2k$vqm$1@build.eclipse.org...
Adrian,
Comments below.
Adrian Price wrote:=20
Hi Ed,
I certainly take your point that one could hand-crank all the subclasses =
to=20
support composition, but wouldn't it be nice if genned base switch =
classes=20
did it for free? Sounds like more work for me though. :-P
[AP] :-)
Here's a revised, cost-freeCost to whom? :-P
[AP] I meant cost-free purely as in no additional instance state and =
no performance hit to regular switch subclasses. Some extra byte code to =
declare the interface, true.
approach that could actually=20
work:
public interface org.eclipse.emf.ecore.util.Switch<T> {
New interfaces aren't entirely free.
T doSwitch(EObject theEObject);
boolean supportsEPackage(EPackage pkg);
Or perhaps isSwitchFor...
[AP] Yes, I think I prefer that.
}
public class XxxSwitch<T> implements Switch<T> {
One would have to rely on models being regenerated to support this. =
That's not a problem, except that many projects are lazy about, or worse =
yet averse to, regenerating. Oh well, too bad for them.
[AP] True enough, but they would only need to regenerate if they =
discovered a need to compose their switches.
public boolean supportsEPackage(EPackage pkg) {
return pkg =3D=3D XxxPackage.eINSTANCE;
}
...
}
/** Example implementation; others possible. */
It would be good to provide a high quality reusable base.
[AP] Agreed. There are clearly refinements required to the example to =
achieve that quality level.
public class ComposedSwitch<T> implements Switch<T> {
private final Switch<T> cacheMiss =3D new Switch<T>() {
public T doSwitch(EObject theEObject) { return null; }
public boolean supportsEPackage(EPackage pkg) { return false; }
};
private final Switch<T>[] delegates;
private final Map<EPackage, Switch<T>> delegatesByPackage =3D new=20
HashMap<EPackage, Switch<T>>();
public ComposedSwitch(Switch<T>... delegates) {
this.delegates =3D delegates.clone();
}
public T doSwitch(EObject theEObject) {
Switch<T> delegate =3D=20
findDelegate(theEObject.eClass().getEPackage());
return delegate =3D=3D null ? null : =
delegate.doSwitch(theEObject);
Would it be better for the composed switch to support the same type of =
logic as the generated switch?
[AP] Yes, agreed. I omitted that logic from the example for the sake =
of brevity (honestly ;-) ).
protected T doSwitch(EClass theEClass, EObject theEObject)
{
if (theEClass.eContainer() =3D=3D modelPackage)
{
return doSwitch(theEClass.getClassifierID(), theEObject);
}
else
{
List<EClass> eSuperTypes =3D theEClass.getESuperTypes();
return
eSuperTypes.isEmpty() ?
defaultCase(theEObject) :
doSwitch(eSuperTypes.get(0), theEObject);
}
}
I suppose the =3D=3D modelPackage should call the new tester method =
and then this entire method could be moved to a base class; that would =
actually save some byte code which might well offset the additional byte =
code. It makes me wonder if an abstract base class shared by generated =
switches and the composed switch instead of a interface might not be =
just as functional... =20
[AP] Good suggestions, both. A shared abstract base class is starting =
to sound better all round.
}
private Switch<T> findDelegate(EPackage pkg) {
Switch<T> delegate =3D delegatesByPackage.get(pkg);
if (delegate =3D=3D null) {
delegate =3D cacheMiss;
I'm not sure I understand the value of cacheMiss given it's converted =
back into null...
[AP] It was a performance optimisation to avoid re-iterating all the =
delegates after having previously determined that none match the package =
in question.
for (Switch<T> sw : delegates) {
if (sw.supportsEPackage(pkg)) {
delegate =3D sw;
break;
}
}
delegatesByPackage.put(pkg, delegate);
[AP] Actually, there is a simpler way to avoid the re-iteration:
private Switch<T> findDelegate(EPackage pkg) {
Switch<T> delegate;
if (delegatesByPackage.containsKey(pkg)) {
delegate =3D delegatesByPackage.get(pkg);
} else {
delegate =3D null;
for (Switch<T> sw : delegates) {
if (sw.supportsEPackage(pkg)) {
delegate =3D sw;
break;
}
}
delegatesByPackage.put(pkg, delegate);
}
return delegate;
}
[AP] Now I'm wondering whether the likely number of delegates even =
warrants such an optimisation - a cache hit involving containsKey(pkg) =
immediately followed by get(pkg) might be more expensive than iterating =
a short delegates list on every cache miss. Especially given that you =
would probably design your composed switches to handle all the packages =
they were likely to encounter, so on cache hit cases you would always =
pay the cost of supporting the cache miss case that would only occur =
once per package, on the first call.
Would it be better for the instance to be thread safe?
[AP] Again, I thought about that but reasoned that since the generated =
model classes are not threadsafe there was no need for the switch class =
to be. However, I suppose the findDelegate(EPackage) method could be =
made synchronized for a small performance penalty.
}
return delegate =3D=3D cacheMiss ? null : delegate;
}
public boolean supportsEPackage(EPackage pkg) {
return findDelegate(pkg) !=3D null;
}
}
It all sounds like quite a bit of work, for me. :-P Oh well, too bad =
for me...
[AP] This was only a suggestion, since I have encountered the =
requirement several times. If you think it has merit and decide to =
implement it... well, I'd be positively delighted! :-) If not, I can =
certainly get by with what I currently have.
Cheers,
Adrian Price
Senior Architect | TIBCO Software, Inc.
"Ed Merks" <Ed.Merks@gmail.com> wrote in message=20
news:h5pfmv$bg3$1@build.eclipse.org...
Adrian,
That's an interesting idea... I wonder though... In the end, switches=20
need to do something specific and useful yet the generated XyzSwitch is=20
effectively useless as is, i.e., it is only useful when one derives from =
it to do specific things with one or more of the case methods. As such, =
when building a specific derived switch that does particular things and=20
hence make sense to compose with other derived switches that do that =
same=20
specific thing, a common interface that "declares" the interesting thing =
they do could be introduced at that point...j
=20
<snip/>=20
------=_NextPart_000_0049_01CA1C0D.A067A4E0
Content-Type: text/html;
charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<HTML><HEAD><TITLE></TITLE>
<META content=3Dtext/html;charset=3DISO-8859-1 =
http-equiv=3DContent-Type>
<META name=3DGENERATOR content=3D"MSHTML 8.00.6001.18812">
<STYLE></STYLE>
</HEAD>
<BODY bgColor=3D#ffffff text=3D#000000>
<DIV><FONT size=3D2 face=3DArial>Ed, thanks for your improvements, =
comments=20
below.</FONT></DIV>
<DIV><FONT size=3D2 face=3DArial></FONT> </DIV>
<DIV><FONT size=3D2 face=3DArial>Cheers,</FONT></DIV>
<DIV><FONT size=3D2 face=3DArial></FONT> </DIV>
<DIV><FONT size=3D2 face=3DArial>--A</FONT></DIV>
<BLOCKQUOTE=20
style=3D"BORDER-LEFT: #000000 2px solid; PADDING-LEFT: 5px; =
PADDING-RIGHT: 0px; MARGIN-LEFT: 5px; MARGIN-RIGHT: 0px"=20
dir=3Dltr>
<DIV>"Ed Merks" <<A=20
href=3D"mailto:Ed.Merks@gmail.com">Ed.Merks@gmail.com</A>> wrote in =
message=20
<A=20
=
href=3D"news:h60h2k$vqm$1@build.eclipse.org">news:h60h2k$vqm$1@build.ecli=
pse.org</A>...</DIV>Adrian,<BR><BR>Comments=20
below.<BR><BR>Adrian Price wrote:=20
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D"">Hi Ed,
I certainly take your point that one could hand-crank all the subclasses =
to=20
support composition, but wouldn't it be nice if genned base switch =
classes=20
did it for free? </PRE></BLOCKQUOTE>
<DIV>Sounds like more work for me though. :-P</DIV>
<DIV>[AP] :-)<BR></DIV>
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D"">Here's a revised, =
cost-free</PRE></BLOCKQUOTE>
<DIV>Cost to whom? :-P</DIV>
<DIV>[AP] I meant cost-free purely as in no additional instance =
state and=20
no performance hit to regular switch subclasses. Some extra byte code =
to=20
declare the interface, true.<BR></DIV>
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D""> approach that could actually=20
work:
public interface org.eclipse.emf.ecore.util.Switch<T> {
</PRE></BLOCKQUOTE>New interfaces aren't entirely free.<BR>
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D""> T doSwitch(EObject theEObject);
boolean supportsEPackage(EPackage pkg);
</PRE></BLOCKQUOTE>
<DIV>Or perhaps isSwitchFor...</DIV>
<DIV>[AP] Yes, I think I prefer that.<BR></DIV>
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D"">}
public class XxxSwitch<T> implements Switch<T> {
</PRE></BLOCKQUOTE>
<DIV>One would have to rely on models being regenerated to support =
this. =20
That's not a problem, except that many projects are lazy about, or =
worse yet=20
averse to, regenerating. Oh well, too bad for them.</DIV>
<DIV>[AP] True enough, but they would only need to regenerate if they=20
discovered a need to compose their switches.<BR></DIV>
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D""> public boolean =
supportsEPackage(EPackage pkg) {
return pkg =3D=3D XxxPackage.eINSTANCE;
}
...
}
/** Example implementation; others possible. */
</PRE></BLOCKQUOTE>
<DIV>It would be good to provide a high quality reusable base.</DIV>
<DIV>[AP] Agreed. There are clearly refinements required to the =
example to=20
achieve that quality level.<BR></DIV>
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D"">public class ComposedSwitch<T> =
implements Switch<T> {
private final Switch<T> cacheMiss =3D new Switch<T>() {
public T doSwitch(EObject theEObject) { return null; }
public boolean supportsEPackage(EPackage pkg) { return false; }
};
private final Switch<T>[] delegates;
private final Map<EPackage, Switch<T>> =
delegatesByPackage =3D new=20
HashMap<EPackage, Switch<T>>();
public ComposedSwitch(Switch<T>... delegates) {
this.delegates =3D delegates.clone();
}
public T doSwitch(EObject theEObject) {
Switch<T> delegate =3D=20
findDelegate(theEObject.eClass().getEPackage());
return delegate =3D=3D null ? null : =
delegate.doSwitch(theEObject);
</PRE></BLOCKQUOTE>
<DIV>Would it be better for the composed switch to support the same =
type of=20
logic as the generated switch?</DIV>
<DIV>[AP] Yes, agreed. I omitted that logic from the example for =
the sake=20
of brevity (honestly ;-) ).<BR></DIV>
<BLOCKQUOTE> protected T doSwitch(EClass theEClass, EObject=20
theEObject)<BR> {<BR> if =
(theEClass.eContainer() =3D=3D=20
modelPackage)<BR> =
{<BR> =20
return doSwitch(theEClass.getClassifierID(),=20
theEObject);<BR> }<BR> =20
else<BR> {<BR> =20
List<EClass> eSuperTypes =3D=20
theEClass.getESuperTypes();<BR> =20
return<BR> =
eSuperTypes.isEmpty()=20
?<BR> =20
defaultCase(theEObject)=20
:<BR> =20
doSwitch(eSuperTypes.get(0), theEObject);<BR> =
}<BR> =20
}<BR></BLOCKQUOTE>
<DIV>I suppose the =3D=3D modelPackage should call the new tester =
method and then=20
this entire method could be moved to a base class; that would actually =
save=20
some byte code which might well offset the additional byte code. =
It=20
makes me wonder if an abstract base class shared by generated switches =
and the=20
composed switch instead of a interface might not be just as=20
functional... </DIV>
<DIV>[AP] Good suggestions, both. A shared abstract base class is =
starting to=20
sound better all round.<BR></DIV>
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D""> }
private Switch<T> findDelegate(EPackage pkg) {
Switch<T> delegate =3D delegatesByPackage.get(pkg);
if (delegate =3D=3D null) {
delegate =3D cacheMiss;
</PRE></BLOCKQUOTE>
<DIV>I'm not sure I understand the value of cacheMiss given it's =
converted=20
back into null...</DIV>
<DIV>[AP] It was a performance optimisation to avoid re-iterating all =
the=20
delegates after having previously determined that none match the =
package in=20
question.</DIV>
<DIV><FONT size=3D2 face=3DArial></FONT><FONT size=3D2 =
face=3DArial></FONT><FONT=20
size=3D2 face=3DArial></FONT><FONT size=3D2 face=3DArial></FONT><FONT =
size=3D2=20
face=3DArial></FONT><BR> </DIV>
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D""> for (Switch<T> sw : =
delegates) {
if (sw.supportsEPackage(pkg)) {
delegate =3D sw;
break;
}
}
delegatesByPackage.put(pkg, delegate);
</PRE></BLOCKQUOTE>
<DIV>[AP] Actually, there is a simpler way to avoid the=20
re-iteration:</DIV>
<DIV><FONT size=3D2 face=3D"Courier New"> private=20
Switch<T> findDelegate(EPackage pkg)=20
{<BR> Switch<T>=20
delegate;<BR> if=20
(delegatesByPackage.containsKey(pkg))=20
=
{<BR> =20
delegate =3D=20
=
delegatesByPackage.get(pkg);<BR>  =
; }=20
else =
{<BR> =20
delegate =3D=20
=
null;<BR> &nbs=
p;=20
for (Switch<T> sw : delegates)=20
=
{<BR> &n=
bsp; =20
if (sw.supportsEPackage(pkg))=20
=
{<BR> &n=
bsp; &n bsp; =20
delegate =3D=20
=
sw;<BR> =
=20
=
break;<BR> &nb=
sp; =20
=
}<BR> =20
=
}<BR> =20
delegatesByPackage.put(pkg,=20
delegate);<BR> =20
}<BR> return=20
delegate;<BR> }</FONT></DIV>
<DIV><FONT size=3D2 face=3DArial>[AP] Now I'm wondering whether the =
likely number=20
of delegates even warrants such an optimisation - a cache hit=20
involving containsKey(pkg) immediately followed by get(pkg) might =
be more=20
expensive than iterating a short delegates list on every cache =
miss.=20
Especially given that you would probably design your composed switches =
to=20
handle all the packages they were likely to encounter, so on cache hit =
cases=20
you would always pay the cost of supporting the cache miss case that =
would=20
only occur once per package, on the=20
first call.</FONT></DIV>
<DIV><FONT size=3D2 face=3DArial></FONT> </DIV>
<DIV>Would it be better for the instance to be thread safe?<BR><FONT =
size=3D2=20
face=3DArial>[AP] Again, I thought about that but reasoned that since =
the=20
generated model classes are not threadsafe there was no need for the =
switch=20
class to be. However, I suppose the findDelegate(EPackage) method =
could be=20
made synchronized for a small performance penalty.</FONT></DIV>
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D""> }
return delegate =3D=3D cacheMiss ? null : delegate;
}
public boolean supportsEPackage(EPackage pkg) {
return findDelegate(pkg) !=3D null;
}
}
</PRE></BLOCKQUOTE>
<DIV>It all sounds like quite a bit of work, for me. :-P Oh =
well, too=20
bad for me...</DIV>
<DIV>[AP] This was only a suggestion, since I have encountered the =
requirement=20
several times. If you think it has merit and decide to implement it... =
well,=20
I'd be positively delighted! :-) If not, I can certainly get by with =
what I=20
currently have.<BR></DIV>
<BLOCKQUOTE cite=3Dmid:h5vc9s$dun$1@build.eclipse.org =
type=3D"cite"><PRE wrap=3D"">Cheers,
Adrian Price
Senior Architect | TIBCO Software, Inc.
"Ed Merks" <A class=3Dmoz-txt-link-rfc2396E =
href=3D"mailto:Ed.Merks@gmail.com"><Ed.Merks@gmail.com></A> wrote =
in message=20
<A class=3Dmoz-txt-link-freetext =
href=3D"news:h5pfmv$bg3$1@build.eclipse.org">news:h5pfmv$bg3$1@build.ecli=
pse.org</A>...
</PRE>
<BLOCKQUOTE type=3D"cite"><PRE wrap=3D"">Adrian,
That's an interesting idea... I wonder though... In the end, switches=20
need to do something specific and useful yet the generated XyzSwitch is=20
effectively useless as is, i.e., it is only useful when one derives from =
it to do specific things with one or more of the case methods. As such, =
when building a specific derived switch that does particular things and=20
hence make sense to compose with other derived switches that do that =
same=20
specific thing, a common interface that "declares" the interesting thing =
they do could be introduced at that point...j
</PRE></BLOCKQUOTE><PRE wrap=3D""><!---->
<snip/>=20
</PRE></BLOCKQUOTE></BLOCKQUOTE></BODY></HTML>
------=_NextPart_000_0049_01CA1C0D.A067A4E0--
|
|
|
| Re: Composing Switches [message #480004 is a reply to message #479983] |
Thu, 13 August 2009 12:47 |
Ed Merks
Messages: 33140
Registered: July 2009 |
Senior Member |
|
|
This is a multi-part message in MIME format.
--------------060706090105070700060602
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Adrian,
Comments below.
Adrian Price wrote:
> Ed, thanks for your improvements, comments below.
>
> Cheers,
>
> --A
>
> "Ed Merks" <Ed.Merks@gmail.com <mailto:Ed.Merks@gmail.com>> wrote
> in message news:h60h2k$vqm$1@build.eclipse.org...
> Adrian,
>
> Comments below.
>
> Adrian Price wrote:
>> Hi Ed,
>>
>> I certainly take your point that one could hand-crank all the subclasses to
>> support composition, but wouldn't it be nice if genned base switch classes
>> did it for free?
> Sounds like more work for me though. :-P
> [AP] :-)
>> Here's a revised, cost-free
> Cost to whom? :-P
> [AP] I meant cost-free purely as in no additional instance state
> and no performance hit to regular switch subclasses. Some extra
> byte code to declare the interface, true.
>> approach that could actually
>> work:
>>
>> public interface org.eclipse.emf.ecore.util.Switch<T> {
>>
> New interfaces aren't entirely free.
>> T doSwitch(EObject theEObject);
>> boolean supportsEPackage(EPackage pkg);
>>
> Or perhaps isSwitchFor...
> [AP] Yes, I think I prefer that.
>> }
>>
>> public class XxxSwitch<T> implements Switch<T> {
>>
> One would have to rely on models being regenerated to support
> this. That's not a problem, except that many projects are lazy
> about, or worse yet averse to, regenerating. Oh well, too bad for
> them.
> [AP] True enough, but they would only need to regenerate if they
> discovered a need to compose their switches.
>> public boolean supportsEPackage(EPackage pkg) {
>> return pkg == XxxPackage.eINSTANCE;
>> }
>> ...
>> }
>>
>> /** Example implementation; others possible. */
>>
> It would be good to provide a high quality reusable base.
> [AP] Agreed. There are clearly refinements required to the example
> to achieve that quality level.
>> public class ComposedSwitch<T> implements Switch<T> {
>> private final Switch<T> cacheMiss = new Switch<T>() {
>> public T doSwitch(EObject theEObject) { return null; }
>> public boolean supportsEPackage(EPackage pkg) { return false; }
>> };
>> private final Switch<T>[] delegates;
>> private final Map<EPackage, Switch<T>> delegatesByPackage = new
>> HashMap<EPackage, Switch<T>>();
>>
>> public ComposedSwitch(Switch<T>... delegates) {
>> this.delegates = delegates.clone();
>> }
>>
>> public T doSwitch(EObject theEObject) {
>> Switch<T> delegate =
>> findDelegate(theEObject.eClass().getEPackage());
>> return delegate == null ? null : delegate.doSwitch(theEObject);
>>
> Would it be better for the composed switch to support the same
> type of logic as the generated switch?
> [AP] Yes, agreed. I omitted that logic from the example for the
> sake of brevity (honestly ;-) ).
>
> protected T doSwitch(EClass theEClass, EObject theEObject)
> {
> if (theEClass.eContainer() == modelPackage)
> {
> return doSwitch(theEClass.getClassifierID(), theEObject);
> }
> else
> {
> List<EClass> eSuperTypes = theEClass.getESuperTypes();
> return
> eSuperTypes.isEmpty() ?
> defaultCase(theEObject) :
> doSwitch(eSuperTypes.get(0), theEObject);
> }
> }
>
> I suppose the == modelPackage should call the new tester method
> and then this entire method could be moved to a base class; that
> would actually save some byte code which might well offset the
> additional byte code. It makes me wonder if an abstract base
> class shared by generated switches and the composed switch instead
> of a interface might not be just as functional...
> [AP] Good suggestions, both. A shared abstract base class is
> starting to sound better all round.
>> }
>>
>> private Switch<T> findDelegate(EPackage pkg) {
>> Switch<T> delegate = delegatesByPackage.get(pkg);
>> if (delegate == null) {
>> delegate = cacheMiss;
>>
> I'm not sure I understand the value of cacheMiss given it's
> converted back into null...
> [AP] It was a performance optimisation to avoid re-iterating all
> the delegates after having previously determined that none match
> the package in question.
>
Ah, I missed that point. There's no reason for it not to be static then...
>
>
>
>> for (Switch<T> sw : delegates) {
>> if (sw.supportsEPackage(pkg)) {
>> delegate = sw;
>> break;
>> }
>> }
>> delegatesByPackage.put(pkg, delegate);
>>
> [AP] Actually, there is a simpler way to avoid the re-iteration:
> private Switch<T> findDelegate(EPackage pkg) {
> Switch<T> delegate;
> if (delegatesByPackage.containsKey(pkg)) {
> delegate = delegatesByPackage.get(pkg);
> } else {
> delegate = null;
> for (Switch<T> sw : delegates) {
> if (sw.supportsEPackage(pkg)) {
> delegate = sw;
> break;
> }
> }
> delegatesByPackage.put(pkg, delegate);
> }
> return delegate;
> }
> [AP] Now I'm wondering whether the likely number of delegates even
> warrants such an optimisation - a cache hit
> involving containsKey(pkg) immediately followed by get(pkg) might
> be more expensive than iterating a short delegates list on every
> cache miss.
>
Misses probably generally indicate a problem. I think this does slow
down the normal case.
>
> Especially given that you would probably design your composed
> switches to handle all the packages they were likely to encounter,
> so on cache hit cases you would always pay the cost of supporting
> the cache miss case that would only occur once per package, on the
> first call.
>
> Would it be better for the instance to be thread safe?
> [AP] Again, I thought about that but reasoned that since the
> generated model classes are not threadsafe there was no need for
> the switch class to be.
>
True, but things like the validator class and the Ecore model instances
themselves are thread safe.
>
> However, I suppose the findDelegate(EPackage) method could be made
> synchronized for a small performance penalty.
>> }
>> return delegate == cacheMiss ? null : delegate;
>> }
>>
>> public boolean supportsEPackage(EPackage pkg) {
>> return findDelegate(pkg) != null;
>> }
>> }
>>
>>
> It all sounds like quite a bit of work, for me. :-P Oh well, too
> bad for me...
> [AP] This was only a suggestion, since I have encountered the
> requirement several times. If you think it has merit and decide to
> implement it... well, I'd be positively delighted! :-) If not, I
> can certainly get by with what I currently have.
>
Please open a bugzilla enhancement request.
>
>> Cheers,
>>
>> Adrian Price
>> Senior Architect | TIBCO Software, Inc.
>>
>> "Ed Merks" <Ed.Merks@gmail.com> wrote in message
>> news:h5pfmv$bg3$1@build.eclipse.org...
>>
>>> Adrian,
>>>
>>> That's an interesting idea... I wonder though... In the end, switches
>>> need to do something specific and useful yet the generated XyzSwitch is
>>> effectively useless as is, i.e., it is only useful when one derives from
>>> it to do specific things with one or more of the case methods. As such,
>>> when building a specific derived switch that does particular things and
>>> hence make sense to compose with other derived switches that do that same
>>> specific thing, a common interface that "declares" the interesting thing
>>> they do could be introduced at that point...j
>>>
>>
>> <snip/>
>>
>>
>>
>
--------------060706090105070700060602
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Adrian,<br>
<br>
Comments below.<br>
<br>
Adrian Price wrote:
<blockquote cite="mid:h60ro3$gdq$1@build.eclipse.org" type="cite">
<title></title>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
<meta name="GENERATOR" content="MSHTML 8.00.6001.18812">
<style></style>
<div><font face="Arial" size="2">Ed, thanks for your improvements,
comments below.</font></div>
<div> </div>
<div><font face="Arial" size="2">Cheers,</font></div>
<div> </div>
<div><font face="Arial" size="2">--A</font></div>
<blockquote
style="border-left: 2px solid rgb(0, 0, 0); padding-left: 5px; padding-right: 0px; margin-left: 5px; margin-right: 0px;"
dir="ltr">
<div>"Ed Merks" <<a moz-do-not-send="true"
href="mailto:Ed.Merks@gmail.com">Ed.Merks@gmail.com</a>> wrote in
message <a moz-do-not-send="true"
href="news:h60h2k$vqm$1@build.eclipse.org">news:h60h2k$vqm$1@build.eclipse.org</a>...</div>
Adrian,<br>
<br>
Comments below.<br>
<br>
Adrian Price wrote:
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap="">Hi Ed,
I certainly take your point that one could hand-crank all the subclasses to
support composition, but wouldn't it be nice if genned base switch classes
did it for free? </pre>
</blockquote>
<div>Sounds like more work for me though. :-P</div>
<div>[AP] :-)<br>
</div>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap="">Here's a revised, cost-free</pre>
</blockquote>
<div>Cost to whom? :-P</div>
<div>[AP] I meant cost-free purely as in no additional instance
state and no performance hit to regular switch subclasses. Some extra
byte code to declare the interface, true.<br>
</div>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> approach that could actually
work:
public interface org.eclipse.emf.ecore.util.Switch<T> {
</pre>
</blockquote>
New interfaces aren't entirely free.<br>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> T doSwitch(EObject theEObject);
boolean supportsEPackage(EPackage pkg);
</pre>
</blockquote>
<div>Or perhaps isSwitchFor...</div>
<div>[AP] Yes, I think I prefer that.<br>
</div>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap="">}
public class XxxSwitch<T> implements Switch<T> {
</pre>
</blockquote>
<div>One would have to rely on models being regenerated to support
this. That's not a problem, except that many projects are lazy about,
or worse yet averse to, regenerating. Oh well, too bad for them.</div>
<div>[AP] True enough, but they would only need to regenerate if
they discovered a need to compose their switches.<br>
</div>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> public boolean supportsEPackage(EPackage pkg) {
return pkg == XxxPackage.eINSTANCE;
}
...
}
/** Example implementation; others possible. */
</pre>
</blockquote>
<div>It would be good to provide a high quality reusable base.</div>
<div>[AP] Agreed. There are clearly refinements required to the
example to achieve that quality level.<br>
</div>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap="">public class ComposedSwitch<T> implements Switch<T> {
private final Switch<T> cacheMiss = new Switch<T>() {
public T doSwitch(EObject theEObject) { return null; }
public boolean supportsEPackage(EPackage pkg) { return false; }
};
private final Switch<T>[] delegates;
private final Map<EPackage, Switch<T>> delegatesByPackage = new
HashMap<EPackage, Switch<T>>();
public ComposedSwitch(Switch<T>... delegates) {
this.delegates = delegates.clone();
}
public T doSwitch(EObject theEObject) {
Switch<T> delegate =
findDelegate(theEObject.eClass().getEPackage());
return delegate == null ? null : delegate.doSwitch(theEObject);
</pre>
</blockquote>
<div>Would it be better for the composed switch to support the same
type of logic as the generated switch?</div>
<div>[AP] Yes, agreed. I omitted that logic from the example for
the sake of brevity (honestly ;-) ).<br>
</div>
<blockquote> protected T doSwitch(EClass theEClass, EObject
theEObject)<br>
{<br>
if (theEClass.eContainer() == modelPackage)<br>
{<br>
return doSwitch(theEClass.getClassifierID(), theEObject);<br>
}<br>
else<br>
{<br>
List<EClass> eSuperTypes = theEClass.getESuperTypes();<br>
return<br>
eSuperTypes.isEmpty() ?<br>
defaultCase(theEObject) :<br>
doSwitch(eSuperTypes.get(0), theEObject);<br>
}<br>
}<br>
</blockquote>
<div>I suppose the == modelPackage should call the new tester
method and then this entire method could be moved to a base class; that
would actually save some byte code which might well offset the
additional byte code. It makes me wonder if an abstract base class
shared by generated switches and the composed switch instead of a
interface might not be just as functional... </div>
<div>[AP] Good suggestions, both. A shared abstract base class is
starting to sound better all round.<br>
</div>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> }
private Switch<T> findDelegate(EPackage pkg) {
Switch<T> delegate = delegatesByPackage.get(pkg);
if (delegate == null) {
delegate = cacheMiss;
</pre>
</blockquote>
<div>I'm not sure I understand the value of cacheMiss given it's
converted back into null...</div>
<div>[AP] It was a performance optimisation to avoid re-iterating
all the delegates after having previously determined that none match
the package in question.</div>
</blockquote>
</blockquote>
Ah, I missed that point. There's no reason for it not to be static
then...<br>
<blockquote cite="mid:h60ro3$gdq$1@build.eclipse.org" type="cite">
<blockquote
style="border-left: 2px solid rgb(0, 0, 0); padding-left: 5px; padding-right: 0px; margin-left: 5px; margin-right: 0px;"
dir="ltr">
<div><br>
</div>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> for (Switch<T> sw : delegates) {
if (sw.supportsEPackage(pkg)) {
delegate = sw;
break;
}
}
delegatesByPackage.put(pkg, delegate);
</pre>
</blockquote>
<div>[AP] Actually, there is a simpler way to avoid the
re-iteration:</div>
<div><font face="Courier New" size="2"> private Switch<T>
findDelegate(EPackage pkg) {<br>
Switch<T> delegate;<br>
if (delegatesByPackage.containsKey(pkg)) {<br>
delegate = delegatesByPackage.get(pkg);<br>
} else {<br>
delegate = null;<br>
for (Switch<T> sw : delegates) {<br>
if (sw.supportsEPackage(pkg)) {<br>
delegate = sw;<br>
break;<br>
}<br>
}<br>
delegatesByPackage.put(pkg, delegate);<br>
}<br>
return delegate;<br>
}</font></div>
<div><font face="Arial" size="2">[AP] Now I'm wondering whether the
likely number of delegates even warrants such an optimisation - a cache
hit involving containsKey(pkg) immediately followed by get(pkg) might
be more expensive than iterating a short delegates list on every cache
miss.</font></div>
</blockquote>
</blockquote>
Misses probably generally indicate a problem. I think this does slow
down the normal case.<br>
<blockquote cite="mid:h60ro3$gdq$1@build.eclipse.org" type="cite">
<blockquote
style="border-left: 2px solid rgb(0, 0, 0); padding-left: 5px; padding-right: 0px; margin-left: 5px; margin-right: 0px;"
dir="ltr">
<div><font face="Arial" size="2"> Especially given that you would
probably design your composed switches to handle all the packages they
were likely to encounter, so on cache hit cases you would always pay
the cost of supporting the cache miss case that would only occur once
per package, on the first call.</font></div>
<div> </div>
<div>Would it be better for the instance to be thread safe?<br>
<font face="Arial" size="2">[AP] Again, I thought about that but
reasoned that since the generated model classes are not threadsafe
there was no need for the switch class to be. </font></div>
</blockquote>
</blockquote>
True, but things like the validator class and the Ecore model instances
themselves are thread safe.<br>
<blockquote cite="mid:h60ro3$gdq$1@build.eclipse.org" type="cite">
<blockquote
style="border-left: 2px solid rgb(0, 0, 0); padding-left: 5px; padding-right: 0px; margin-left: 5px; margin-right: 0px;"
dir="ltr">
<div><font face="Arial" size="2">However, I suppose the
findDelegate(EPackage) method could be made synchronized for a small
performance penalty.</font></div>
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap=""> }
return delegate == cacheMiss ? null : delegate;
}
public boolean supportsEPackage(EPackage pkg) {
return findDelegate(pkg) != null;
}
}
</pre>
</blockquote>
<div>It all sounds like quite a bit of work, for me. :-P Oh well,
too bad for me...</div>
<div>[AP] This was only a suggestion, since I have encountered the
requirement several times. If you think it has merit and decide to
implement it... well, I'd be positively delighted! :-) If not, I can
certainly get by with what I currently have.<br>
</div>
</blockquote>
</blockquote>
Please open a bugzilla enhancement request.<br>
<blockquote cite="mid:h60ro3$gdq$1@build.eclipse.org" type="cite">
<blockquote
style="border-left: 2px solid rgb(0, 0, 0); padding-left: 5px; padding-right: 0px; margin-left: 5px; margin-right: 0px;"
dir="ltr">
<blockquote cite="mid:h5vc9s$dun$1@build.eclipse.org" type="cite">
<pre wrap="">Cheers,
Adrian Price
Senior Architect | TIBCO Software, Inc.
"Ed Merks" <a moz-do-not-send="true" class="moz-txt-link-rfc2396E"
href="mailto:Ed.Merks@gmail.com"><Ed.Merks@gmail.com></a> wrote in message
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="news:h5pfmv$bg3$1@build.eclipse.org">news:h5pfmv$bg3$1@build.eclipse.org</a>...
</pre>
<blockquote type="cite">
<pre wrap="">Adrian,
That's an interesting idea... I wonder though... In the end, switches
need to do something specific and useful yet the generated XyzSwitch is
effectively useless as is, i.e., it is only useful when one derives from
it to do specific things with one or more of the case methods. As such,
when building a specific derived switch that does particular things and
hence make sense to compose with other derived switches that do that same
specific thing, a common interface that "declares" the interesting thing
they do could be introduced at that point...j
</pre>
</blockquote>
<pre wrap=""><!---->
<snip/>
</pre>
</blockquote>
</blockquote>
</blockquote>
</body>
</html>
--------------060706090105070700060602--
Ed Merks
Professional Support: https://www.macromodeling.com/
|
|
| |
Goto Forum:
Current Time: Wed Apr 24 16:43:53 GMT 2024
Powered by FUDForum. Page generated in 0.03828 seconds |
] 
Search
Help
Register
Login
Home
