I'm very uncomfortable about adding this method as is. Firstly, as Peter says (in a different thread), naming this method `map()` is inconsistent with `Stream` and `Optional`. Adding `map()` to `String` would be: public String map(Function<Character, Character>); Not that such a method would be particularly useful!
Secondly, the premise of the method is effectively an alternative to the language feature "extension methods". While the motivation in https://bugs.openjdk.java.net/browse/JDK-8203703 and https://dzone.com/articles/making-java-fluent-api-more-flexible makes some sense, it is clearly equally applicable to all types - `String` is not special here. An isolated change here just looks odd and ill thought through - a big mistake for the core `String` class. My preference is for the change to not proceed without consideration of the broader context. The change should not be added without a commitment to also add the same method to Stream and Optional as a minimum, something that would make the addition part of a more consistent broader design. Doing this would also mandate a different method name. FWIW, were such a method added to java.time.* as the OP suggested, it would enable pluggable conversions: java.util.Date ju = LocalDate.now() .plusDays(2) .apply(MyUtility::convertToOldDate); Clearly there is some value to the above in terms of abstraction. However, there would also be problems making the best choice of types. Does it go on the concrete LocalDate or the interface Temporal. Its a problem just like String vs CharSequence. TLDR, this change needs to be part of a broader context to be acceptable (something that would inform the best name). Without that broader context, I'm against this change. Stephen On Mon, 12 Nov 2018 at 14:05, Jim Laskey <james.las...@oracle.com> wrote: > > updated webrev: > http://cr.openjdk.java.net/~jlaskey/8203442/webrev-02/index.html > <http://cr.openjdk.java.net/~jlaskey/8203442/webrev-02/index.html> > > On Sep 21, 2018, at 7:42 AM, Remi Forax <fo...@univ-mlv.fr> wrote: > > > > ----- Mail original ----- > >> De: "Alan Bateman" <alan.bate...@oracle.com> > >> À: "Jim Laskey" <james.las...@oracle.com>, "core-libs-dev" > >> <core-libs-dev@openjdk.java.net> > >> Envoyé: Vendredi 21 Septembre 2018 12:22:42 > >> Objet: Re: RFR - JDK-8203442 String::transform (Code Review) > > > >> On 18/09/2018 18:52, Jim Laskey wrote: > >>> Please review the code for String::transform. The goal is to provide a > >>> String > >>> instance method to allow function application of custom transformations > >>> applied > >>> to an instance of String. > >>> > >>> webrev: http://cr.openjdk.java.net/~jlaskey/8203442/webrev/index.html > >>> jbs: https://bugs.openjdk.java.net/browse/JDK-8203442 > >>> csr: https://bugs.openjdk.java.net/browse/JDK-8203703 > >> I hate to bring up naming but if I have a Stream<String> or > >> Optional<String> then I'll use the "map" method to apply the mapping > >> function. Has this already been through the naming bike shed and it > >> settled on "transform"? Maybe a different name was chosen after looking > >> at code that would use it in stream operations? Maybe "transform" was > >> used as the argument to the function is always text? I'm not interested > >> in re-opening any discussions, just trying to see if options are > >> captured in a mail somewhere. > >> > >> I'm also wondering if this is general enough to be defined by > >> CharSequence. Retrofitting CharSequence is always problematic and never > >> done lightly but I'm just wondering if it has been discussed and > >> dismissed. I don't think it could be done at a later time, at least not > >> without changing the input type parameter so that the mapping function > >> is Function<? super CharSequence, R> rather than Function<String, R>. > > > > the main issue is that a lot of utility methods take a String as parameter, > > not a CharSequence :( > > and Function<? super String, R> is a supertype not a subtype of Function<? > > super CharSequence, R>. > > > >> > >> -Alan > > > > Rémi >
