RFR: 8015317: Optional.filter, map, and flatMap

[Zhong Yu]

I guess your methods should return Box<A> to be comparable to
Stream/Optional/CompletionStage cases.

On Mon, Jul 15, 2013 at 2:17 AM, Jed Wesley-Smith <jed at wesleysmith.io> wrote:
> ignore me, you do actually need both ? extends on the type constructor and
> the inner type – dunno what I was thinking.
>
>
> On 15 July 2013 13:02, Jed Wesley-Smith <jed at wesleysmith.io> wrote:
>>
>> I'm not entirely sure that is a problem, have a look at the following:
>>
>> https://gist.github.com/jedws/5993596#file-variancetest-java
>>
>> it is only the one with a covariance annotation on the parameter that
>> fails…
>>
>>
>> On 15 July 2013 12:52, Zhong Yu <zhong.j.yu at gmail.com> wrote:
>>>
>>> Another example of possible missing a wildcard
>>>
>>>
>>> http://gee.cs.oswego.edu/dl/jsr166/dist/docs/java/util/concurrent/CompletionStage.html#thenCompose%28java.util.function.Function%29
>>>
>>>    thenCompose(Function<? super T,? extends CompletionStage<U>> fn)
>>>
>>> should be
>>>
>>>    thenCompose(Function<? super T,? extends CompletionStage<? extends U>>
>>> fn)
>>>
>>> The problem is probably wide spread, and we need a tool to find these
>>> mistakes.
>>>
>>> Zhong Yu
>>>
>>>
>>> On Sun, Jul 14, 2013 at 8:04 AM, Jed Wesley-Smith <jed at wesleysmith.io>
>>> wrote:
>>> > (accidentally didn't post to the list)
>>> >
>>> >> You probably know that the example provided is not completed ported to
>>> > work with our Optional implementation,
>>> >
>>> > It should be, for the example I wrote an Optional that is final and
>>> > should
>>> > be otherwise identical. It should certainly be fairly easy for any
>>> > committer to try. If you can make it work without the ? extends
>>> > Optional
>>> > I'd love an explanation of how.
>>> >
>>> >> and fugue works around the type system with Option as abstract class.
>>> >
>>> > As I've tried to explain, this isn't about the implementation of the
>>> > container class, but how covariance works with a parameterised class.
>>> >
>>> > We originally had the non-, but in a discussion with Brian he alerted
>>> > us to
>>> > the fact that the signature was wrong. We hastily fixed it:
>>> >
>>> >
>>> > https://bitbucket.org/atlassian/fugue/commits/9eca663326a5baeb8f23974732ec585d5627a05c
>>> >
>>> > To further demonstrate, I give you a minimal example of a final
>>> > Optional
>>> > implementation that does not compile for this test:
>>> >
>>> > https://gist.github.com/jedws/5993596#file-gistfile1-java-L57
>>> >
>>> > cheers,
>>> > jed.
>>> >
>>> >
>>> >
>>> > On 14 July 2013 15:02, Henry Jen <henry.jen at oracle.com> wrote:
>>> >
>>> >>  I think I understand what you are saying. However, unless we make
>>> >> Optional not final, the extends part just doesn't matter.
>>> >>
>>> >> You probably know that the example provided is not completed ported to
>>> >> work with our Optional implementation, and fugue works around the type
>>> >> system with Option as abstract class.
>>> >>
>>> >> Cheers,
>>> >> Henry
>>> >>
>>> >>  On Jul 13, 2013, at 4:35 PM, Jed Wesley-Smith
>>> >> <jed at wesleysmith.io><jed at wesleysmith.io>wrote:
>>> >>
>>> >>  I did supply a test that you can use to try it.
>>> >>
>>> >>  What we are talking about is whether type Box<Parent> is
>>> >> substitutable
>>> >> by Box<Child> in the contravariant position. Intuitively we think we
>>> >> only
>>> >> need Box<? extends Parent> because we only care about the type
>>> >> parameter,
>>> >> but the type – as you point out – is actually different. Box<Parent>
>>> >> is not
>>> >> "inherited by" Box<Child>.
>>> >>
>>> >>  Specifically, if we have a consuming Box, and we replace it with a
>>> >> Box
>>> >> of a more specific type parameter, we could attempt feed the more
>>> >> general
>>> >> type into it, ie. a Box<Child> isn't going to appreciate having Parent
>>> >> fed
>>> >> to it. This is why covariance and mutable containers don't mix well,
>>> >> and
>>> >> why Java's covariant arrays are problematic.
>>> >>
>>> >>  In this situation we have an immutable container, and we can
>>> >> substitute
>>> >> the type of our container with one of a more specific type, as it will
>>> >> only
>>> >> ever supply a value – and a value of Child will suffice as a Parent.
>>> >> So,
>>> >> for this case we need a Box that is substitutable, and therefore we
>>> >> need to
>>> >> add the covariance to our box.
>>> >>
>>> >>  <? extends Box> is simply adding covariance to our Box type.
>>> >>
>>> >>  For a much better explanation than I can give about this, see this
>>> >> excellent post describing generics in Scala, which – apart from have
>>> >> declaration-site variance and using [A] in place of <A> – generally
>>> >> follow
>>> >> the same pattern:
>>> >>
>>> >>
>>> >>
>>> >> http://termsandtruthconditions.herokuapp.com/blog/2012/12/29/covariance-and-contravariance-in-scala/
>>> >>
>>> >>  cheers,
>>> >> jed.
>>> >>
>>> >>
>>> >> On 14 July 2013 04:49, Henry Jen <henry.jen at oracle.com> wrote:
>>> >>
>>> >>> I think the type you talking about here is Optional<? extends U>
>>> >>> instead
>>> >>> of ? extends Optional<U>.
>>> >>>
>>> >>>  IIRC, Optional<? extends U> is not a subtype of Optional<U>, just
>>> >>> like
>>> >>> any other Collection class. List<Child> is not a List<Parent>.
>>> >>>
>>> >>>  Cheers,
>>> >>> Henry
>>> >>>
>>> >>>
>>> >>>  On Jul 13, 2013, at 3:15 AM, Jed Wesley-Smith <jed at wesleysmith.io>
>>> >>> wrote:
>>> >>>
>>> >>>  > The ? extends Optional is unnecessary in flatMap as Optional is
>>> >>> final.
>>> >>>
>>> >>>  interestingly enough, it actually is.
>>> >>>
>>> >>>  try the following test:
>>> >>>
>>> >>>  class OptionalTest {
>>> >>>   class Parent {};
>>> >>>
>>> >>>    class Child extends Parent {};
>>> >>>
>>> >>>    @Test public void covariantReturn() {
>>> >>>     Optional<Parent> some = some(new Parent());
>>> >>>     Function<Parent, Optional<Child>> f = new Function<Parent,
>>> >>> Optional<Child>>() {
>>> >>>       @Override public Optional<Child> apply(Parent p) {
>>> >>>         return some(new Child());
>>> >>>       }
>>> >>>     };
>>> >>>     Optional<Parent> mapped = some.<Parent> flatMap(f);
>>> >>>     assertThat(mapped.get(), notNullValue());
>>> >>>   }
>>> >>> }
>>> >>>
>>> >>>  adapted from the fugue test suite:
>>> >>>
>>> >>>
>>> >>>
>>> >>> https://bitbucket.org/atlassian/fugue/src/96a65067fb7aaf1edae1bffa07167a5865cbebec/src/test/java/com/atlassian/fugue/OptionTest.java#cl-155
>>> >>>
>>> >>>  The point to remember is that Optional<Child> is a type and as such
>>> >>> is
>>> >>> actually a subtype of Optional<Parent> –  and therefore requires a
>>> >>> covariant return.
>>> >>>
>>> >>>  cheers,
>>> >>> jed.
>>> >>>
>>> >>>
>>> >>>
>>> >>>
>>> >>> On 13 July 2013 04:15, Mike Duigou <mike.duigou at oracle.com> wrote:
>>> >>>
>>> >>>> The ? extends Optional is unnecessary in flatMap as Optional is
>>> >>>> final.
>>> >>>> Otherwise this looks good.
>>> >>>>
>>> >>>> Mike
>>> >>>>
>>> >>>> On Jul 5 2013, at 14:37 , Henry Jen wrote:
>>> >>>>
>>> >>>> > Hi,
>>> >>>> >
>>> >>>> > Please review the webrev at
>>> >>>> >
>>> >>>> > http://cr.openjdk.java.net/~henryjen/ccc/8015317.0/webrev/
>>> >>>> >
>>> >>>> > Which adds following method to Optional,
>>> >>>> >
>>> >>>> > public static <T> Optional<T> ofNullable(T value) {}
>>> >>>> > public Optional<T> filter(Predicate<? super T> predicate) {}
>>> >>>> > public<U> Optional<U> map(Function<? super T, ? extends U> mapper)
>>> >>>> > {}
>>> >>>> > public<U> Optional<U> flatMap(Function<? super T, ? extends
>>> >>>> Optional<U>>
>>> >>>> > mapper) {}
>>> >>>> >
>>> >>>> > Also included is some cleanup on javadoc.
>>> >>>> >
>>> >>>> > Cheers,
>>> >>>> > Henry
>>> >>>>
>>> >>>>
>>> >>>>
>>> >>>
>>> >>>
>>> >>
>>> >>
>>> >
>>
>>
>