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/raw_files/new/ 6304578.2/raw_files/new/
diff -r
6304578.1/raw_files/new/src/java.base/share/classes/java/lang/Class.java
6304578.2/raw_files/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/raw_files/new/src/java.base/share/classes/java/lang/reflect/Executable.java
6304578.2/raw_files/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 ", "")));
