Bug 313005

Comment 1 Thomas Schindl 2010-06-08 10:27:29 EDT

Oleg? Any objections?

Comment 2 Thomas Schindl 2010-06-10 03:34:04 EDT

sorry for bothering but any news/thoughts on this as well :-)

Comment 3 Oleg Besedin 2010-06-10 09:33:21 EDT

(In reply to comment #0)
> In bug 284940 IEclipseContext got generified and has Java5 set as BREE. This
> means that org.eclipse.e4.core.services also depends on Java5 as BREE and we
> can introduce generics there too.

Umm... I am not sure. I think John's idea was that this bundle will be able to run on Java 4 using downcompile from Java 5. I don't know how that will work in practice if it has a non-optional dependency on a bundle that requires Java5. John, can you clarify this?
 
> The attached patch introduces generics, updates the BREE and fixes the
> build.properties

We usually include schema files in the source "includes", but not in the binary "includes" in the build.properties. Is there some reason to include it in the binary build?

Comment 4 Oleg Besedin 2010-06-10 09:35:46 EDT

(In reply to comment #0)
> The attached patch introduces generics, updates the BREE and fixes the
> build.properties

What's the value of using

 	public <T> T call(Object object, ...)

instead of

	public Object call(Object object, ...) ?

Usually the type T will appear in some other place, but not in those methods.

Comment 5 John Arthorne 2010-06-10 09:56:27 EDT

(In reply to comment #0)
> Created an attachment (id=168629) [details]
> Patch to introduce generics
> 
> In bug 284940 IEclipseContext got generified and has Java5 set as BREE. This
> means that org.eclipse.e4.core.services also depends on Java5 as BREE and we
> can introduce generics there too.

You can already use generics in org.eclipse.e4.core.services. It is configured to use Java 5 syntax but compile to java 1.4 class files. There are already other classes in that bundle using generics.  The changes to the manifest and compiler settings in this patch aren't needed.

Comment 6 Thomas Schindl 2010-06-10 10:23:49 EDT

You don't need to case(In reply to comment #4)
> (In reply to comment #0)
> > The attached patch introduces generics, updates the BREE and fixes the
> > build.properties
> 
> What's the value of using
> 
>      public <T> T call(Object object, ...)
> 
> instead of
> 
>     public Object call(Object object, ...) ?
> 
> Usually the type T will appear in some other place, but not in those methods.

You don't need to cast your own:

public Object call(Object object, ...)
--------------------------------------
MyType c = (MyType)call(...)

public <T> T call(Object object, ...)
-------------------------------------
MyType c = call(...)

Comment 7 Thomas Schindl 2010-06-10 10:26:35 EDT

(In reply to comment #4)
> (In reply to comment #0)
> > The attached patch introduces generics, updates the BREE and fixes the
> > build.properties
> 
> What's the value of using
> 
>      public <T> T call(Object object, ...)
> 
> instead of
> 
>     public Object call(Object object, ...) ?
> 
> Usually the type T will appear in some other place, but not in those methods.

(In reply to comment #6)
> You don't need to case(In reply to comment #4)
> > (In reply to comment #0)
> > > The attached patch introduces generics, updates the BREE and fixes the
> > > build.properties
> > 
> > What's the value of using
> > 
> >      public <T> T call(Object object, ...)
> > 
> > instead of
> > 
> >     public Object call(Object object, ...) ?
> > 
> > Usually the type T will appear in some other place, but not in those methods.
> 
> You don't need to cast your own:
> 
> public Object call(Object object, ...)
> --------------------------------------
> MyType c = (MyType)call(...)
> 
> public <T> T call(Object object, ...)
> -------------------------------------
> MyType c = call(...)

On a second thought I don't think this is necessary because it might make it more subtle to find incorrect class casts. So I'm fine with closing this as WONTFIX

Comment 8 Oleg Besedin 2010-06-10 10:43:02 EDT

I to think that IContributionFactory needs to be cleaned up, just not necessarily in the exactly the way proposed here. I'll close this bug; there is bug 299553 that will serve as a reminder to clean up IContributionFactory.