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

[Stephen Colebourne]

I see from Twitter (!!!) that this has been pushed. This appears to have
happened without this thread coming to a clear conclusion, particularly wrt
criticism of transform() as a suitable method name in the broader context.

I also do not think that the code review was completed correctly and in
public, which I find concerning.

The two public threads are:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-November/056574.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-November/056592.html

The last webrev had the name map() and no further webrev was published that
I can see. I can see no explicit public approvals of the change from
reviewers. And plenty of concern about the method name, especially map()
but also transform().

While I'm well aware of the danger of public bikeshedding, I also think
that adding methods to `String` deserves more care than this has been
shown. In my view the change should be reverted, and retargetted to Java 13
to allow for a proper conclusion to the discussion.

For info, AWS SDK v2 has chosen applyMutation() for a similar concept:
https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/utils/builder/SdkBuilder.html#applyMutation-java.util.function.Consumer-

Stephen


On Wed, 14 Nov 2018, 14:18 Stephen Colebourne <scolebourne at joda.org wrote:

> On Tue, 13 Nov 2018 at 15:44, Brian Goetz <brian.goetz at oracle.com> wrote:
> > Yes, we know :)  But we don’t have any current plans to do that, nor
> use-site extension methods, nor does it seem likely to come to the top of
> the language priority list very soon.  So its not a choice between |> and
> .transform(); it’s a choice between transform() and nothing.  Picking
> transform() seems pretty good here.
> >
> > Stephen raised the issue of a “broader context”; indeed, the intention
> of adding a transform() method here was that it would be feasible to add to
> other types for which it was suitable.  String is an obvious candidate for
> “do it first”, but this is within a broader context.
>
> I'd be more comforted if there was a commitment to add the method to
> Stream and Optional in Java 12 or 13.
>
> I also agree with Remi that `transform()` is a poor name for Stream,
> and thus it is a poor name for String. I doubt there is a perfect
> name, process() or apply() seem like good candidates.
>
> Stephen
>