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

[Tomasz Linkowski]

Hi,

My 2 cents:

1) I agree that `String.map` is a really unfortunate name - Peter explained
it very well! I'd rather wait than have `String.map`.

2) I absolutely agree that `String`, `Stream`, and `Optional` should share
the name.

3) I prefer `Stream.transform` to `Stream.chain` but I understand it might
be confusing to newbies.
BTW. Stream-like `transform` has already been used by Lukas Eder in his
`Seq`:
https://www.jooq.org/products/jOO
λ/javadoc/0.9.14/org/jooq/lambda/Seq.html#transform-java.util.function.Function-

4) I'm not convinced by `asInputTo` (too many words) nor by `applyMutation`
(confusing, like Tagir said).

On a side note: regardless of the name, I bet we'll see some
`string.transform(String::toLowerCase)` or `stream.chain(s->s.map(mapper))`
:)

Regards,
Tomasz Linkowski


On Mon, Dec 3, 2018 at 12:32 PM Tagir Valeev <amaembo at gmail.com> wrote:

> +1 to Stephen and Remi. I personally see no reason to hurry up with
> this API: unlike trimming/indenting methods this one doesn't look
> crucial for raw string literals. I don't see that this method blocks
> anything else.
>
> > For info, AWS SDK v2 has chosen applyMutation() for a similar concept:
>
> Unfortunately this name serves well only for mutable objects, like
> builders. For String it looks confusing.
>
> With best regards,
> Tagir Valeev.
> On Sat, Dec 1, 2018 at 5:27 AM Remi Forax <forax at univ-mlv.fr> wrote:
> >
> > I fully agree with Stephen.
> >
> > Rémi
> >
> > ----- Mail original -----
> > > De: "Stephen Colebourne" <scolebourne at joda.org>
> > > À: "core-libs-dev" <core-libs-dev at openjdk.java.net>
> > > Envoyé: Vendredi 30 Novembre 2018 12:06:23
> > > Objet: Re: RFR - JDK-8203442 String::transform (Code Review)
> >
> > > 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
>


-- 
Pozdrawiam,
Tomasz Linkowski