Hi Stuart

On 01/12/2011 08:06 AM, Stuart Marks wrote:
Hi Max,

Here, finally, is an updated webrev for 7008713. Like the other updates,
this is the automated diamond conversion but with diamonds removed from
assignment statements. This is mostly diamonds used in variable
initializers. I think there's one in a return statement in there too.

You mean this one?

     public static <A,B> Pair<A,B> of(A a, B b) {
-        return new Pair<A,B>(a,b);
+        return new Pair<>(a,b);
     }

I wonder if it should be included. The reason why people don't like "<>" in non-initializer, which I guess, is that it's too faraway from the declaration and people want to read the full generics parameters as a reminder. This is also true for a return statement when the method declaration is normally faraway from it.

Of course, in this particular case, the return line is very near to the return type declaration, so it doesn't seem too bad. Just don't know how your automated conversion deals with other cases.


I've adjusted whitespace that ended up around the diamond operator
itself. I think there's a pretty strong convention not to have any space
within or around a diamond. There are several places where I made
changes like the following:

new HashMap<> () ===> new HashMap<>()

Do you also change the declaration?

  Map <String> map ===>  Map<String> map

In src/share/classes/sun/security/tools/KeyTool.java, I see

@@ -153,11 +153,11 @@
...
- private List <String> ids = new ArrayList <String> (); // used in GENCRL
-    private List <String> v3ext = new ArrayList <String> ();
+    private List<String> ids = new ArrayList<>();   // used in GENCRL
+    private List<String> v3ext = new ArrayList<>();

so, "List <String> v3ext" is changed to "List<String> v3ext".

but, in the same file,

@@ -3873,8 +3873,7 @@
                         break;
                     case 2:     // EKU
                         if(value != null) {
-                            Vector <ObjectIdentifier> v =
-                                    new Vector <ObjectIdentifier>();
+                            Vector <ObjectIdentifier> v = new Vector<>();
                             for (String s: value.split(",")) {
                                 int p = oneOf(s,
                                         "anyExtendedKeyUsage",

Here, there's still a whitespace in "Vector <ObjectIdentifier>".

There are also lines in this file with only the declaration:

127:    private Set<Pair <String, String>> providers = null;
499:            providers = new HashSet<Pair <String, String>> (3);
663:        for (Pair <String, String> provider: providers) {


For their webrevs both Sean and Brad had asked me to tweak whitespace
after the comma in the list of generic type arguments. For example, like
the following:

Map<String,Object> ===> Map<String, Object>

However, I did *not* make such changes in this webrev. I didn't know
whether you would have wanted such changes; there are rather a lot of
them to change, and it's also not clear to me whether it actually would
have improved the consistency of the spacing in this section of code.
But, let me know if you'd like me to adjust this whitespace as well.

My style was no whitespace after "," but probably it's not good. Now I think <K,V> is OK but <String, String> is better. Page 130 of "Effective Java 2nd version" seems to support this.

Since this code change is about coinification, I really don't care about whitespace styles here.

Thanks
Max


Here's a link to the updated webrev.

http://cr.openjdk.java.net/~smarks/reviews/7008713/webrev.1/

Thanks!

s'marks

Reply via email to