RFR: refactorings for Java 5 and 7 features
Joe Darcy
joe.darcy at oracle.com
Tue Dec 17 09:48:49 PST 2013
On 12/17/2013 09:04 AM, Brian Goetz wrote:
> As part of a code cleanup effort, we're updating our codebase to take
> advantage of features added in previous Java versions, which we
> haven't been as good about as we'd like. This is a first batch of
> changes, but won't be the last.
>
> For this batch, I've mostly restricted myself to those that can be
> automatically detected and fixed using IntelliJ, though some were done
> manually because they required more complex restructuring.
>
> The webrev:
> http://cr.openjdk.java.net/~briangoetz/JDK-8030245/webrev/
>
> has proposed changes for JDK-8030245 (try-with-resources and
> multi-catch), JDK-8030253 (strings in switch), and JDK-8030262
> (foreach loops).
>
>
Very happy to see these kind of changes coming to langtools :-)
A few comments / notes of caution.
---
a/src/share/classes/com/sun/tools/doclets/internal/toolkit/util/DocFile.java
+++
b/src/share/classes/com/sun/tools/doclets/internal/toolkit/util/DocFile.java
@@ -132,19 +132,15 @@
* Copy the contents of another file directly to this file.
*/
public void copyFile(DocFile fromFile) throws IOException {
- InputStream input = fromFile.openInputStream();
- OutputStream output = openOutputStream();
- try {
+ try (OutputStream output = openOutputStream();
+ InputStream input = fromFile.openInputStream()) {
byte[] bytearr = new byte[1024];
int len;
while ((len = input.read(bytearr)) != -1) {
output.write(bytearr, 0, len);
}
- } catch (FileNotFoundException exc) {
- } catch (SecurityException exc) {
- } finally {
- input.close();
- output.close();
+ }
+ catch (FileNotFoundException | SecurityException exc) {
}
}
The refactoring is fine, but this whole method can probably be replaced
by a line or two of cod using the nio.2 API from JDK 7, perhaps an
adaptation to one of the methods in java.nio.file.Files.
Throughout, multi-catch makes the code so much nicer!
Some of the changes involve rely on implicit boxing or unboxing, like below:
--- a/src/share/classes/com/sun/tools/javac/parser/JavacParser.java
+++ b/src/share/classes/com/sun/tools/javac/parser/JavacParser.java
@@ -698,9 +698,9 @@
// error already reported in scanner
n = Float.NaN;
}
- if (n.floatValue() == 0.0f && !isZero(proper))
+ if (n == 0.0f && !isZero(proper))
error(token.pos, "fp.number.too.small");
- else if (n.floatValue() == Float.POSITIVE_INFINITY)
+ else if (n == Float.POSITIVE_INFINITY)
error(token.pos, "fp.number.too.large");
else
t = F.at(pos).Literal(TypeTag.FLOAT, n);
@@ -717,9 +717,9 @@
// error already reported in scanner
n = Double.NaN;
}
- if (n.doubleValue() == 0.0d && !isZero(proper))
+ if (n == 0.0d && !isZero(proper))
error(token.pos, "fp.number.too.small");
- else if (n.doubleValue() == Double.POSITIVE_INFINITY)
+ else if (n == Double.POSITIVE_INFINITY)
error(token.pos, "fp.number.too.large");
else
t = F.at(pos).Literal(TypeTag.DOUBLE, n);
I'm a bit cautious towards these changes, not being fully certain
off-hand of all the boxing vs unboxing priorities and the hazard of an
== being done on objects rather than a equals comparison of values.
The strings in switch and for loop changes look good.
Thanks,
-Joe
More information about the compiler-dev
mailing list