Community
Participate
Working Groups
Currently we use Class.forName to detect if the IDE bundle is available... this is evil, we should be using package admin.
adding context
Created attachment 79652 [details] mylyn/context/zip
can you be more clear bout the downsides of Class.forName in this context?
Right now Jeff we have some horrible code in the log view that does this: // TODO this isn't the best way to check... we should be smarter and use package admin // check to see if org.eclipse.ui.ide is available Class.forName("org.eclipse.ui.ide.IDE"); //$NON-NLS-1$ // check to see if org.eclipse.core.filesystem is available Class.forName("org.eclipse.core.filesystem.IFileStore"); //$NON-NLS-1$ action = new OpenIDELogFileAction(this); } catch (ClassNotFoundException e) { action = new Action() { public void run() { if (fInputFile.exists()) { Job job = getOpenLogFileJob(); job.setUser(false); job.setPriority(Job.SHORT); job.schedule(); } } }; } We should instead check for ide/filesystem bundles (or packages) using PackageAdmin as a cleaner and more consistent way (we do this in PDE elsewhere). Jeff, did you see "Class.forName" and immediately jump out of your seat? That's how I pictured it :)!
After talking with Tom, his suggestion was to keep the Class.forName instead of using PackageAdmin. It seems that using PackageAdmin would incur additional unnecessary overhead for this case. Nice brain storming Chris, I liked the creative thinking :)
(In reply to comment #4) > Jeff, did you see "Class.forName" and immediately jump out of your seat? That's > how I pictured it :)! Not quite but you surely can get my attention by including "Class.forName" in something :-) As far as I can see the only real downside to using this approach for testing availability of function is that it does cause the requested class to be loaded and that may in turn cause some bundle activation. If you do the test somewhere near where you actually want to use the function the downsides can be minimized.
> As far as I can see the only real downside to using this approach for testing > availability of function is that it does cause the requested class to be loaded > and that may in turn cause some bundle activation. If you do the test > somewhere near where you actually want to use the function the downsides can be > minimized. Exactly. More importantly, the code snippet above is so 90's java development ;)
(In reply to comment #7) > > Exactly. > > More importantly, the code snippet above is so 90's java development ;) > I would like you to create a patch with the PackageAdmin approach and see how much better the code looks ;-) A simple Class.forName vs. ServiceTracker create/open/getservice/check for null/do PackageAdmin stuff/close tracker in finally vs. get ServiceReference from BundleContext/check for null/BundleContext.getService/check for null/do PackageAdmin stuff/unget service in finally. As Jeff said the concern about activating bundles too early is minimized by the fact that you perform the check right before you perform the action which would have activated the bundles anyway.
