Bugzilla – Full Text Bug Listing
| Summary: | IEclipseContext#set(Class,Object) should be really typesafe | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Thomas Schindl <tom.schindl> | ||||||
| Component: | E4 | Assignee: | Project Inbox <e4.runtime-inbox> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | bokowski, ob1.eclipse | ||||||
| Version: | unspecified | ||||||||
| Target Milestone: | --- | ||||||||
| Hardware: | PC | ||||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
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.
Oleg? Any objections? (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? (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. 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.
Applied updated patch to CVS Head. Tom, thanks for the great idea! (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. |
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)