5

I have a relatively simple looking problem that I am trying to solve. There doesn't seem to be an intuitive way to do this or, I am missing something here.

Consider this method to find the main image and if none exists, return first image-

public Image findMainImage(Collection<? extends Image> images) {
    if (images == null || images.isEmpty()) return null
    return images.stream()
                 .filter(Image::isMain)
                 .findFirst()
                 .orElse(images.iterator().next())
}

I get an error - orElse(capture<? extends Image>) in Optional cannot be applied

Any direction on this would be great.

Radiodef
  • 37,180
  • 14
  • 90
  • 125
sanz
  • 1,069
  • 1
  • 10
  • 26
  • 3
    This would probably work if it was just a `Collection`, but the way generics work, `Optional.orElse` can only accept a `T`. – Louis Wasserman Apr 24 '15 at 22:44
  • Can you change the method signature to `public T findMainImage(Collection images)` – Misha Apr 24 '15 at 22:49
  • @misha I get how it works by changing the signature, I am more interested in understanding the reasoning behind this. – sanz Apr 24 '15 at 23:00
  • 1
    If you use a return value of your collection with a wildcard subtype (`? extends ...`), without binding it to a definitive type (e.g. by assigning it to a variable of the type `Image`, you'll have a `? extends Image` type, where as the `orElse` method expects an `Image` because of type inference. Java generics are a pain, especially when wildcards are involved. – thrau Apr 24 '15 at 23:00
  • 1
    @sanz The reasoning is **you** know that `? extends Image` which parametrizes the stream is the same `? extends Image` that comes out of `images.iterator().next()` but the compiler doesn't know. By altering the signature and giving a name to collection parameter you can then prove to the compiler that it's the same type in both places. – Misha Apr 24 '15 at 23:54

2 Answers2

7

One way to fix it is to use a type parameter:

public <I extends Image> I findMainImage(Collection<I> images) {
    if (images == null || images.isEmpty()) return null;
    return images.stream()
                 .filter(Image::isMain)
                 .findFirst()
                 .orElse(images.iterator().next());
}

Because then (to the compiler) the Optional definitely has the same type argument as images.

And we could use that as a capturing helper if we wanted:

public Image findMainImage(Collection<? extends Image> images) {
    return findMainImageHelper( images );
}

private <I extends Image> I findMainImageHelper(Collection<I> images) {
    // ...
}

Personally, I would just use the generic version because then you can do e.g.:

List<ImageSub> list = ...;
ImageSub main = findMainImage( list );

Basically...the reasoning for why it doesn't compile originally is to keep you from doing something like this:

public Image findMainImage(
    Collection<? extends Image> images1,
    Collection<? extends Image> images2
) {
    return images1.stream()
                  .filter(Image::isMain)
                  .findFirst()
                  .orElse(images2.iterator().next());
}

And in the original example the compiler doesn't need to determine the fact that both the Stream and the Iterator come from the same object. Two separate expressions that reference the same object get captured to two separate types.

Radiodef
  • 37,180
  • 14
  • 90
  • 125
2

Suppose you have a List<? extends Number>. You could not add any number to this list, because it could be a List<Integer> and the number you are attempting to add could be a Float.

For the same reason, the method orElse(T t) of Optional<T> requires a T. Since the Optional in question is an Optional<? extends Image>, the compiler can't be sure that images.iterator().next() is of the right type.

I got this to compile by putting in .map(t -> (Image) t):

return images.stream()
             .filter(Image::isMain)
             .findFirst()
             .map(t -> (Image) t)
             .orElse(images.iterator().next());

In fact, for some reason I can't understand, it works even without the cast. Just using

.map(t -> t)

seems to make this work.

Paul Boddington
  • 37,127
  • 10
  • 65
  • 116
  • 1
    I assume that you really have `.map` before `findFirst`. The type of `t` is the upper bound of the collection, `Image`, so whether you cast `t` to `Image` or not`, it's an `Image`, and the `Stream` returned is now a `Stream`, instead of `Stream extends Image>`. The `Optional` is now `Optional` instead of `Optional extends Image>`, and that allows you to pass an `Image` to `orElse`. – rgettman Apr 24 '15 at 23:01
  • @rgettman `map` can go in either position. `Optional` has a method `map` too, so I have mapped an `Optional extends Image>` to an `Optional`. – Paul Boddington Apr 24 '15 at 23:05
  • I didn't even realize that `Optional` had a `map` method also. There's the saying "You learn something new every day", and I just learned something new. – rgettman Apr 24 '15 at 23:08
  • @rgettman Either way, it's odd that you don't need the cast, no? How does the compiler know the desired type is `Image`? – Paul Boddington Apr 24 '15 at 23:09
  • Both `Stream`'s and `Map`'s methods take a `Function super T, ? extends R>`. The compiler might be inferring the return type of the `Function` based on the parameter type of the `orElse` method, but I can't tell for sure. – rgettman Apr 24 '15 at 23:22
  • `.map(Image.class::cast)` – dkarp Apr 24 '15 at 23:37
  • Since the elements are already instances of `Image`, you don’t need a type cast when using `map`, i.e. you can use `.map(t->t)` or `.map(Function.identity())` – Holger Apr 27 '15 at 10:24
  • @Holger I get that. What I find slightly odd is that you can get away with just `.map(t->t)`. – Paul Boddington Apr 27 '15 at 11:01
  • Hmm, doesn’t work on my jdk. However, inferring the type argument as written explicitly in my examples, wouldn’t be wrong, so it doesn’t look odd to me. I knew examples, where `.map(t -> t)` can make a desired type transition even without explicit types, before… Just recall that when you have a `class A extends B`, `Function f=t->t;` is valid… – Holger Apr 27 '15 at 11:16