Community
Participate
Working Groups
Created attachment 191564 [details] ClassUtil refactoring Doing some work on RWT internals I found the class org.eclipse.rwt.internal.util.ClassUtil. I agree very much with the objective of that class. Providing an abstraction for loading and creating an instance of a certain type reflectivly is used at several locations in RWT. I also agree with the idea of introducing an special exception class to hide all the exception types involved by the reflection stuff. But I think in case problems occur in the reflectivly invoked constructor itself treatment of that exceptions should be a little bit more sophisticated. In case of runtime exceptions I don't want any nested stacktrace at all. I want the stacktrace to point me directly to that line of the constructor that causes the problem. In case of checked exceptions I can not rethrow this exceptions of course. But having a ClassInstantiationException wrapping a InvocationTargetException that contains the original problem seem also a little bit overwhelming to me. I would prefer the InvocationTargetException to be eliminated. So I've created an additional test and changed the exception handling accordingly. Please have a look at the attached patch with those changes. Last but not least I think the name ClassUtil is too unspecific. InstanceCreationUtil or something like this would describe the purpose of the class more precisely. What do you think?
+1 for the enhance exception handling. Regarding the name, ClassUtil might not be as specific as InstanceCreationUtil but in tandem with the newInstance() methods it gives a clear understanding what it is for - in my eyes. Moreover I don't like that InstanceCreationUtil#newInstance() carries the word 'instance' twice. How about ReflectionUtil?
Thanks for this improvement, Frank. Applied the patch to CVS HEAD and adapted ServiceHandler_Test accordingly. Regarding the name, I also find InstanceCreationUtil#newInstance() a bit verbosely and redundant. I'm actually fine with ClassUtil, but think that ReflectionUtil would also be ok. Would it help to change the method name to createInstance() or even createNewInstance() ? After all it's a good pattern for method names to contain a verb that indicates what the method does.
Ralf, thanks for applying the patch. Regarding the name I did not ment to kick off a longterm discussion. If we don't find a striking name within a blink of an eye, we should leave it as ClassUtil. Rüdiger told me that he thought of using ClassUtil#newInstance as analogy to Class#newInstance. From that point of view I think we can live with it as it is... (In reply to comment #2) > Thanks for this improvement, Frank. Applied the patch to CVS HEAD and adapted > ServiceHandler_Test accordingly. > > Regarding the name, I also find InstanceCreationUtil#newInstance() a bit > verbosely and redundant. I'm actually fine with ClassUtil, but think that > ReflectionUtil would also be ok. Would it help to change the method name to > createInstance() or even createNewInstance() ? After all it's a good pattern > for method names to contain a verb that indicates what the method does.
