Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 313939 - IEclipseContext#set(Class,Object) should be really typesafe
Summary: IEclipseContext#set(Class,Object) should be really typesafe
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: E4 (show other bugs)
Version: unspecified   Edit
Hardware: PC Mac OS X - Carbon (unsup.)
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-21 12:35 EDT by Thomas Schindl CLA
Modified: 2012-12-13 15:00 EST (History)
2 users (show)

See Also:


Attachments
Patch (1.76 KB, patch)
2010-05-21 12:35 EDT, Thomas Schindl CLA
no flags Details | Diff
Updated patch (2.65 KB, patch)
2010-06-08 16:16 EDT, Oleg Besedin CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Schindl CLA 2010-05-21 12:35:38 EDT
Created attachment 169523 [details]
Patch

This method is currently defined as:

public void set(Class<?> clazz, Object object) 

which allows one to write someone something like this:
-------8<-------
ctx.set(My.class,"Bla")
-------8<-------
I guess the reason for this is that this was directly ported from the set(String,Object) which also allows to associate an IContextFunction with it.

I think though that the set(Class<?> clazz, Object object) is a convenience method for typesafety and if someone wants this special feature they should use the string version.

So I'd propose we define it like this:

public <T> void set(Class<T> clazz, T object)
Comment 1 Thomas Schindl CLA 2010-05-21 12:41:24 EDT
One more typesafety idea in conjunction with IEclipseContext/IContextFunction.

Why not redefining IContextFunction like this:

---------8<---------
IContextFunction<T> {
   public T compute(IEclipseContext context);
}
---------8<---------

and then adding a type safe set-method in IEclipseContext:
---------8<---------
IEclipseContext {
  public void set(Class<T> clazz, IContextFunction<T>);
}
---------8<---------

For all special cases I think people can use set(String,Object) which doesn't use any type checks.
Comment 2 Thomas Schindl CLA 2010-06-08 10:26:26 EDT
Oleg? Any objections?
Comment 3 Boris Bokowski CLA 2010-06-08 10:40:19 EDT
(In reply to comment #0)
> public <T> void set(Class<T> clazz, T object)

Looks good to me.

The ContextFunction change looks good, too, but would a change like this have to be coordinated on e4-dev to minimize merge problems with ongoing development?
Comment 4 Thomas Schindl CLA 2010-06-08 10:43:27 EDT
(In reply to comment #3)
> (In reply to comment #0)
> > public <T> void set(Class<T> clazz, T object)
> 
> Looks good to me.
> 
> The ContextFunction change looks good, too, but would a change like this have
> to be coordinated on e4-dev to minimize merge problems with ongoing
> development?

I don't think so because the only thing happening is that you'll get a warning when using it in a none-generic way but if Oleg is ok with the change proposed I could certainly write to e4-dev about this change.
Comment 5 Oleg Besedin CLA 2010-06-08 16:16:14 EDT
Created attachment 171469 [details]
Updated patch 

The IEclipseContext#set() change looks like a very good idea; I also would like to udpate IEclipseContext#modify() method in the same manner.

The IContextFunction change: I'd prefer to avoid adding specialized methods to IEclipseContext. The dependency injection is the preferred method with IContextFunction mostly used internally by the e4 mechanisms.
Comment 6 Oleg Besedin CLA 2010-06-08 16:17:37 EDT
Applied updated patch to CVS Head. 

Tom, thanks for the great idea!
Comment 7 Thomas Schindl CLA 2010-06-08 16:31:41 EDT
(In reply to comment #5)
> Created an attachment (id=171469) [details]
> Updated patch 
> 
> The IEclipseContext#set() change looks like a very good idea; I also would like
> to udpate IEclipseContext#modify() method in the same manner.
> 
> The IContextFunction change: I'd prefer to avoid adding specialized methods to
> IEclipseContext. The dependency injection is the preferred method with
> IContextFunction mostly used internally by the e4 mechanisms.

You mean contribution a context-function through DeclarativeServices right because I'm relying heavily on context functions in my custom e4 code.

But I'm fine with keeping the IEclipseContext-API as small as possible.