Hi Remi,
Catching up on email, at least to my eye, the current version is easier
to read than the proposed refactoring in both cases.
I wouldn't expect this code to be performance critical so I'm inclined
to leave the code as-is unless a performance motivation is found.
Thanks for the comments,
-Joe
On 1/16/2019 1:21 AM, Remi Forax wrote:
Hi Joe,
in j.l.Class, in methodToString, can you change
sb.append(getName() + "." + name + "(");
to
sb.append(getName()).append(".").append(name).append('(');
because using the concatenation inside a StringBuilder creates another
StringBuilder (java.lang doesn't use the StringConcatFactory).
also
return typeVar.getName() + " extends " +
Arrays.stream(bounds)
.map(Type::getTypeName)
.collect(Collectors.joining(" & "));
can be written
return Arrays.stream(bounds)
.map(Type::getTypeName)
.collect(Collectors.joining(" & ", typeVar.getName().concat(" extends "),
""));
which also avoid the creation of an intermediary StringBuilder (at the cost of
the code being less readable).
(in that case, you have also to change the same code in Executable).
regards,
Rémi
----- Mail original -----
De: "joe darcy" <joe.da...@oracle.com>
À: "Stuart Marks" <stuart.ma...@oracle.com>
Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net>
Envoyé: Mercredi 16 Janvier 2019 04:05:05
Objet: Re: JDK 13 RFR of JDK-8217000: Refactor Class::methodToString
Good catch on the 3-argument joining in this case; I'll push with that
amendment.
Thanks for the review,
-Joe
On 1/15/2019 3:19 PM, Stuart Marks wrote:
On 1/14/19 7:41 PM, Joe Darcy wrote:
PS And for good measure, made analogous changes in Executable.java:
http://cr.openjdk.java.net/~darcy/8217000.1/
Thanks for following up on this. Overall, looks good. One point:
114 sb.append('(');
115
116 sb.append(Arrays.stream(parameterTypes)
117 .map(Type::getTypeName)
118 .collect(Collectors.joining(",")));
119
120 sb.append(')');
I think you can use the 3-arg form of joining() here, since the prefix
and suffix are included even if the stream is empty.
s'marks
