RFR - JDK-8203442 String::transform (Code Review)

Stuart Marks stuart.marks at oracle.com
Tue Dec 4 18:36:11 UTC 2018


Hi everybody,

I've finally caught up with all of this. I see that several people are surprised 
by the way this has turned out. As the first-named reviewer on this changeset, I 
felt I should try to figure out what happened.

While this API point stands on its own, this is really part of Jim's RSL work 
[1] which includes several API additions to String, and which will likely have a 
significant effect on how String literals are used in Java code.

Jim started code review [2] and CSR review [3] for this proposal back in 
September. There was a gap of several weeks, followed by a revised proposal on 
12 Nov [4][5] that changed the method's name from "transform" to "map".

There was further discussion over the next couple days; in particular there were 
some objections to the method name "map". On 26 Nov Jim pushed the changeset 
with the name changed back to "transform".

 From reading just the messages on the mailing list, I can certainly see why 
people were surprised. It looked like there was an issue open for discussion, 
yet Jim went ahead and pushed the revised change anyway. What happened?

Given that the primary open issue was about naming, and such mailing list 
discussions can go on for an arbitrarily long time, Jim asked Brian off-list for 
a decision on this. The results were:

1) the name for this method on String should be "transform";

2) adding library methods is a reasonable way for the platform to provide this 
capability (as opposed to various language enhancements that might be 
contemplated); and

3) the intent is to colonize the name "transform" for this operation on this and 
other classes, such as List, Optional, etc.  (It may well be the case that there 
is no name that is a good fit for both Stream and for everything else, given 
Stream's highly stylized API; we can consider the tradeoffs between consistency 
and clarity when we get to that particular one.)

The primary thing that went wrong here was that Brian and Jim neglected to 
circle back to the list to record this decision. This was an oversight.

There could and should have been better communication about this. Brian, Jim, or 
I should have documented the results of this decision and followed up on the 
mailing list. None of us did. Sorry about that.

What else should be done here?

One thing is that we (I) can make a better effort to keep up on emails, though 
the volume -- on a wide variety of topics -- is significant.

Another point is that issues that are raised on the mailing list are often 
discussed and resolved off the mailing list. While we make significant and 
ongoing efforts to write up relevant offline discussions for public consumption 
(see, for example, see the recent writeup on nullable values [6]), sometimes 
things will fall through the cracks.

Finally, we should also record that this particular decision sets a precedent 
for the use of the name "transform" for methods that do similar things on other 
classes. To this end, I've updated this bug with a note about "transform":

   JDK-8140283 [7] add method to Stream that facilitates fluent chaining of 
other methods

and I've also filed the following RFE:

   JDK-8214753 [8] (opt) add Optional::transform, allowing an arbitrary 
operation on an Optional

s'marks

[1] http://openjdk.java.net/jeps/326

[2] http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055532.html

[3] http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055533.html

[4] http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-November/056574.html

[5] http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-November/056592.html

[6] 
http://mail.openjdk.java.net/pipermail/valhalla-spec-experts/2018-November/000784.html

[7] https://bugs.openjdk.java.net/browse/JDK-8140283

[8] https://bugs.openjdk.java.net/browse/JDK-8214753



More information about the core-libs-dev mailing list