I have picked my first Exception in this project.
And unfortunatly i picked InternalError which seems to be used quite
often. :-)
I tried to change all initCause calls to the new Constructor and found
many places where there is code like this:
throw new InternalError(e.toString());
or
throw new InternalError(e.getMessage());
or
throw new InternalError("Lorem ipsum:"+e);
These ones are candidates for cleanup too, but i want to discuss if
someone sees a good argument to not to change to
throw new InternalError(e.getMessage(),e);
or
throw new InternalError("Lorem ipsum",e);
Also i don't completely understand why Throwable(Throwable cause) uses
cause.toString() to fill the message instead of cause.getMessage().
Can someone explain to me why?
Couldn't it be
if (cause == null) {
detailMessage = null;
}else {
String message = cause.getMessage();
detailMessage = (message==null ? null : cause.toString());
}
instead?
- Sebastian
Am 08.08.2011 21:18, schrieb Sebastian Sickelmann:
Thanks for encouraging me.
I opened a bug [1] at bugs.openjdk.java.net. Hope that is a right
first step to getting started.
I wanted to investigate the calls to initCause in jdk-source first to
pick the most valueable Exceptions first.
I thinks there are many exceptions out of scope of core-libs. How to
handle these ones. I think i will
start with exceptions from core-libs.
Is there a pattern that says me exceptions belongs to which
openjdk-project?
- Sebastian
[1] https://bugs.openjdk.java.net/show_bug.cgi?id=100201
Am 08.08.2011 19:04, schrieb Joe Darcy:
Hello.
On 8/8/2011 7:24 AM, Sebastian Sickelmann wrote:
Looks good to me.
But i have one question:
Why there are two ways to plumb the causes of an exception?
The history of this situation is covered in older versions of the
javadoc of Throwable:
http://download.oracle.com/javase/6/docs/api/java/lang/Throwable.html
In many exceptions-classes you can use a constuctor-level
argument to specify the cause, and in some classes you must use the
initCause method.
Is it: "When you often need a cause at creation-time than we got
a specializes constructor." ?
I found 500+ lines jdk/src/share/classes/**/*.java where i found
line like
XYZException iae = new XYZException();
iae.initCause(e);
throw iae;
I am searching a minor project for my second Contribution to
OpenJDK to learn more about the developement / review process.
Is it worthwhile to look at this issue and maybe refactor some
of the often used Exceptions to accept a cause at construction-time?
I would say such a project would be helpful; for context, this topic
came up a few months ago on core-libs in the context of another code
review:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-June/007005.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-June/007008.html
Cheers,
-Joe
- Sebastian
Am 08.08.2011 07:24, schrieb Joe Darcy:
Hello.
Please review this small fix, developed after I went through my
open bug list; patch below, webrev at:
6380161 (reflect) Exception from newInstance() not chained to
cause.
http://cr.openjdk.java.net/~darcy/6380161.0/
Several call to initCause are added to plumb up caused-by exception
chains to exception types without constructors taking a cause. In
addition, multi-catch is added in on location.
I looked for similar issues in Constructor, Method, and Executable,
but there weren't any.
Thanks,
-Joe
--- old/src/share/classes/java/lang/Class.java 2011-08-07
22:21:58.000000000 -0700
+++ new/src/share/classes/java/lang/Class.java 2011-08-07
22:21:58.000000000 -0700
@@ -349,7 +349,8 @@
});
cachedConstructor = c;
} catch (NoSuchMethodException e) {
- throw new InstantiationException(getName());
+ throw (InstantiationException)
+ new
InstantiationException(getName()).initCause(e);
}
}
Constructor<T> tmpConstructor = cachedConstructor;
@@ -973,7 +974,8 @@
descriptor = (String) enclosingInfo[2];
assert((name != null && descriptor != null) || name
== descriptor);
} catch (ClassCastException cce) {
- throw new InternalError("Invalid type in enclosing
method information");
+ throw (InternalError)
+ new InternalError("Invalid type in enclosing
method information").initCause(cce);
}
}
@@ -1239,7 +1241,8 @@
try {
return
getName().substring(enclosingClass.getName().length());
} catch (IndexOutOfBoundsException ex) {
- throw new InternalError("Malformed class name");
+ throw (InternalError)
+ new InternalError("Malformed class
name").initCause(ex);
}
}
@@ -2954,9 +2957,8 @@
}
// These can happen when users concoct enum-like classes
// that don't comply with the enum spec.
- catch (InvocationTargetException ex) { return null; }
- catch (NoSuchMethodException ex) { return null; }
- catch (IllegalAccessException ex) { return null; }
+ catch (InvocationTargetException |
NoSuchMethodException |
+ IllegalAccessException ex) { return null; }
}
return enumConstants;
}
