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