Review Request: 8001533: Java launcher must launch JavaFX applications
Kumar Srinivasan
kumar.x.srinivasan at oracle.com
Fri Nov 16 16:56:23 UTC 2012
Mandy,
Thanks Mandy!, that tip cleaned up the code quite a bit, it is generally
looking
a lot better.
David,
One minor fix the while loop can be converted to a for loop making it
slightly more compact, But I am fine either way.
- Class<?> sc = mainClass.getSuperclass();
- while (sc != null) {
+ for (Class<?> sc = mainClass.getSuperclass(); sc != null;
+ sc = sc.getSuperclass()) {
if (sc.getName().equals(JAVAFX_APPLICATION_CLASS_NAME)) {
return true;
}
- sc = sc.getSuperclass();
}
Steve,
the case of jfxrt.jar not being on the class path, we currently pass
vacuously. Instead,
I think we can have a couple of negative tests, this way we can ensure
the launcher
behavior and error messages under these conditions. We can do this as a
bug fix in M6.
Thanks
Kumar
>>>> L496-517 somewhat duplicates the logic added for FX in the
>>>> getMainClassFromJar method. Have you considered some refactoring
>>>> work you could do to simplify the fix since I think once you get
>>>> the classname of the entry point (either from a JAR or command-line
>>>> and with and without the static void main method), the logic is
>>>> essentially the same. To elaborate, I see that FXHelper.launchName
>>>> L707-725 is needed mainly to give a useful error message. When
>>>> you find the classname of the entry point, perhaps you can load
>>>> the class and catch any linkage error and determine if it's caused
>>>> by the absence of FX runtime and output an appropriate error.
>>>> If the main class is successfully loaded, then proceed with
>>>> L496-517 (or something like that). Just an idea you can explore.
>>> Yes, I do feel that especially in the -jar case there is some repetition. The trouble is the ambiguity of ClassNotFoundException.
>>>
>>> I'll poke at it and see what I can come up with.
>> That's good.
>> Mandy
> I cleaned it up quite a bit, I think it looks a lot better now:
> http://cr.openjdk.java.net/~ddehaven/8001533/webrev.1/
>
> The comments still need some attention, I'll get that first thing on the morrow.
>
> -DrD-
>
More information about the core-libs-dev
mailing list