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