JDK 12 RFR of JDK-6304578: (reflect) toGenericString fails to print bounds of type variables on generic methods (original) (raw)

Vicente Romero vicente.romero at oracle.com
Thu Nov 1 16:50:25 UTC 2018

last iteration looks good to me,

Vicente

On 10/25/18 5:47 PM, joe darcy wrote:

Hi Peter,

Coming back to this review after my Code One activities this year have run their course...

On 10/17/2018 3:07 PM, Peter Levart wrote: Hi Joe,

On 10/17/2018 09:16 PM, joe darcy wrote: PS In response to some off-list feedback, an updated webrev uses a stream-ier implementation:

http://cr.openjdk.java.net/~darcy/6304578.1/ I suggest further niceing: - if you make typeVarBounds() static, you could use it in Stream.map() as a method reference. - in Class.java line 285, you could also use Type::getTypeName method reference - in Class.java line 269, wouldn't the output be nicer if the delimiter in joining collector was ", " instead of "," (with a space after comma)? The same for Executable. Revised webrev generally taking those suggestions up at: http://cr.openjdk.java.net/~darcy/6304578.2/ Diff of relevant parts of prior version below (screening out unrelated changes to Class made in the interim). The behavior with respect to using a comma as a separator is part of the spec of these methods and I don't want to alter that as part of this change. Thanks, -Joe diff -r 6304578.1/rawfiles/new/  6304578.2/rawfiles/new/ diff -r 6304578.1/rawfiles/new/src/java.base/share/classes/java/lang/Class.java 6304578.2/rawfiles/new/src/java.base/share/classes/java/lang/Class.java 268c268 <         sb.append(Stream.of(typeparms).map(t -> typeVarBounds(t)). --- > sb.append(Stream.of(typeparms).map(Class::typeVarBounds). 279c279 <     String typeVarBounds(TypeVariable<?> typeVar) { --- >     static String typeVarBounds(TypeVariable<?> typeVar) { 285c285 <         Stream.of(bounds).map(e -> e.getTypeName()). --- >         Stream.of(bounds).map(Type::getTypeName). 3413c3413,3414 <         StringJoiner sj = new StringJoiner(", ", getName() + "." +_ _name + "(", ")");_ _---_ _>         StringBuilder sb = new StringBuilder(); >     sb.append(getName() + "." + name + "("); 3415,3420c3416,3420 <             for (int i = 0; i < argTypes.length; i++) {_ _<                 Class<?> c = argTypes[i]; <                 sj.add((c == null) ? "null" : c.getName());_ _<             }_ _<         }_ _<         return sj.toString();_ _---_ _>         Stream.of(argTypes).map(c -> {return (c == null) ? "null" : c.getName();}). >         collect(Collectors.joining(",")); >     } >     sb.append(")"); >         return sb.toString(); diff -r 6304578.1/rawfiles/new/src/java.base/share/classes/java/lang/reflect/Executable.java 6304578.2/rawfiles/new/src/java.base/share/classes/java/lang/reflect/Executable.java 116c116 <         sb.append(Stream.of(parameterTypes).map(p -> p.getTypeName()). --- > sb.append(Stream.of(parameterTypes).map(Type::getTypeName). 122c122 <         sb.append(Stream.of(exceptionTypes).map(e -> e.getTypeName()). --- > sb.append(Stream.of(exceptionTypes).map(Type::getTypeName). 137c137 <     String typeVarBounds(TypeVariable<?> typeVar) { --- >     static String typeVarBounds(TypeVariable<?> typeVar) { 143c143 <         Stream.of(bounds).map(e -> e.getTypeName()). --- >         Stream.of(bounds).map(Type::getTypeName). 156c156 <         sb.append(Stream.of(typeparms).map(t -> typeVarBounds(t)). --- > sb.append(Stream.of(typeparms).map(Executable::typeVarBounds). 176,180c176,177 <                 StringJoiner joiner = new StringJoiner(",", " throws_ _", "");_ _<                 for (Type exceptionType : exceptionTypes) {_ _<                     joiner.add(exceptionType.getTypeName());_ _<                 }_ _<                 sb.append(joiner.toString());_ _---_ _> sb.append(Stream.of(exceptionTypes).map(Type::getTypeName). >               collect(Collectors.joining(",", " throws ", "")));

More information about the core-libs-dev mailing list