Add Optional.presentInstances

[GoogleCodeExporter]

It would be handy to filter iterables of optionals for those with a value 
present.

Not sure about selecting those without a value, that might be unnecessary (not 
to say totally useless). But then again, one could just negate the predicate 
that tests for presence of a value.

Original issue reported on code.google.com by j...@nwsnet.de on 2 Aug 2011 at 3:23

[GoogleCodeExporter]

Original comment by fry@google.com on 4 Aug 2011 at 5:10

• Changed title: Optional.getAllPresentValuesFrom
• Changed state: Accepted

[GoogleCodeExporter]

We've heard a few requests for such a static method that would accept 
Iterable<Optional<T>> and return Iterable<T>.  Naming suggestions welcome 
(obviously we don't really intend to use the method name currently in the 
summary line).

Original comment by kevinb@google.com on 4 Aug 2011 at 8:39

[GoogleCodeExporter]

Original comment by kevinb@google.com on 4 Aug 2011 at 8:39

• Added labels: Type-Enhancement

[GoogleCodeExporter]

Well, I originally thought about putting the method in `Predicates`. In case of 
`Optional`, I came up with `value[s]Present` and `selectPresent` so far.

Note: While I do think that "filtering" is basically a good name, I am often 
confused if, say, a method named `filterNonEmpty` would "collect"/"select"/keep 
only non-empty values (think collections) or if it would "filter out" (as in 
"drop") non-empty values. So maybe this is a good time to think about whether a 
more clear name for such cases should be established and used uniformly for 
future methods' names.

Original comment by j...@nwsnet.de on 5 Aug 2011 at 12:05

[GoogleCodeExporter]

Oh, I overlooked that you suggested to not just select the optionals with a 
value present, but also extract their values and return an iterable of those 
instead, too. Yeah, that's great and as such `Predicates` is not the right 
place (as the [public] need for the predicate I originally requested is no 
longer given).

Original comment by j...@nwsnet.de on 5 Aug 2011 at 12:24

[GoogleCodeExporter]

If Optional implemented Iterator, couldn't you do this using Iterables.concat 
(c.f. issue 669)?

I.e. given Iterable<Optional<T>> optionals
Iterable<T> values = Iterables.concat(optionals);

Original comment by douglasv...@gmail.com on 3 Oct 2011 at 9:32

[GoogleCodeExporter]

If it would implement `Iterator`, one would have to use `Iterators.concat`. Or 
`Iterable` with `Iterables.concat`.

In any case, I'm feeling a strong dislike of thinking of an `Optional` - which 
contains a single value (I consider a collection as a single value, too) at 
most - as something iterable. However, I admit that one can see an `Optional` 
as a container/collection with only zero or one values. Still, especially the 
suggestion in issue #669 to use a `for` loop to retrieve its potential single 
value is totally misguided IMHO.

As a consequence, I only see a utility method as a clean way to collect the 
values of multiple `Optional`s.

Original comment by j...@nwsnet.de on 3 Oct 2011 at 12:19

[GoogleCodeExporter]

I believe that's what Scala does. The Option ( 
http://www.scala-lang.org/api/rc/scala/Option.html ) class is actually a 
collection (containing 0 or 1 element). This lets them manipulate Options using 
flatMap / filter / foreach.

http://www.stackoverflow.com/questions/1059776/scala-iterablemap-vs-iterableflat
map/1060400#1060400

"flatMap turns a List[Option[A]] into List[A], with any Option that drills down 
to None, removed. This is a key conceptual breakthrough for getting beyond 
using null."

We discussed the use of flatMap / Optional on StackOverflow last week: 
http://stackoverflow.com/questions/7204632/filtering-lists-of-generic-types/7478
379 . I believe such a method would be useful, letting us transform/filter 
iterables at the same time (using a Function that returns an Optional).

I'm not sure if the right approach is to think of Optional as a Collection 
(using Iterables.concat() to flatten an Iterable<Optional>), or if we should 
simply provide a simple utility method that flattens Optionals.

Original comment by nev...@gmail.com on 3 Oct 2011 at 5:11

[GoogleCodeExporter]

Well, it does look good in Scala (at least the one-liner) and makes sense to 
me. But this is Java and I can't expect my co-workers to use a[n explicit] loop 
(isn't this what FP is trying to avoid/get away from/make unnecessary?) or even 
a transform and filter around each tiny method I return an `Optional` from (and 
I wouldn't myself, too) as it's way too verbose.

Just for the record: A strong point for me for using `Optional` is that I can 
use a compiler-checked return type in a method signature that also makes clear 
that the result cannot be `null`, even though no actual value must be returned. 
Unlike method arguments, I don't know of a possibility to use annotations to 
indicate that (similar to @NotNull etc.).

Original comment by j...@nwsnet.de on 4 Oct 2011 at 7:51

[GoogleCodeExporter]

I'm unclear on how the Iterable<Optional<T>> ends up in the system in the first 
place.  Is there a reason for an API to return such a type?  If not, it sounds 
like someone is building it up even though he knows he's going to convert it to 
an Iterable<T>.  Should we be solving this problem earlier in the program by 
adding to Optional an asSet() or addToCollection(Collection) method, as others 
have proposed?

Original comment by cpov...@google.com on 4 Oct 2011 at 2:19

• Changed state: Triaged

[GoogleCodeExporter]

The use case I had in mind, is when you transform an Iterable using a function 
whose return is optional. You are only interested in values that were present.

For example:

public List<Person> loadPersons(List<Long> personIds) {
    Function<Long, Optional<Person>> loadPersonFunction = new Function<Long, Optional<Person>>() {
        @Override
        public Optional<Person> apply(@Nullable Long id) {
            // attempts to load a Person, returning "absent" if none found
        }
    };
    return ImmutableList.copyOf(Optional.getAllPresentValuesFrom(Iterables.transform(personIds, loadPersonFunction)));
}

addToCollection() is good for imperative programming (e.g. in a for-each loop), 
but it wouldn't work in this case.

asSet() could work, but we'd have to use a Function<Long, Set<Person>> instead 
of Function<Long, Optional<Person>>, which feels weird.

Having Optional implement Iterable might actually be the cleanest way... We 
could then simply do:

return ImmutableList.copyOf(Iterables.concat(Iterables.transform(personIds, 
loadPersonFunction)));

Original comment by nev...@gmail.com on 4 Oct 2011 at 9:48

[GoogleCodeExporter]

Got it, thanks.

Original comment by cpov...@google.com on 4 Oct 2011 at 9:49

[GoogleCodeExporter]

Original comment by cpov...@google.com on 4 Oct 2011 at 9:49

• Changed state: Accepted

[GoogleCodeExporter]

Original comment by kevinb@google.com on 5 Oct 2011 at 5:02

• Added labels: Milestone-Release11

[GoogleCodeExporter]

Iterables.getPresent, perhaps?

Original comment by wasserman.louis on 7 Nov 2011 at 7:35

[GoogleCodeExporter]

Sounds ok to me.

That said, I personally have a slight aversion towards to giving the "get" 
prefix to methods that are not getters.

Also, all the existing `get*` methods in `Iterables` return a single value, not 
an iterable.

Thus, avoiding the ambiguous "filter", I'd go with "selectPresent", 
"collectPresent" or maybe even "pickPresent" (while "retainPresent" and 
"removeAbsent" sound familiar with regard to collections, which are also 
modified in-place; also seen in `Iterables`).

Original comment by j...@nwsnet.de on 15 Nov 2011 at 5:01

[GoogleCodeExporter]

We've got this in labs internally as two methods in Optionals.java:

public static <T> void addIfPresent(Collection<T> collection, Optional<? 
extends T> optional);
public static <T> Iterable<T> extractPresent(Iterable<Optional<T>> optionals);

Both don't have much usage yet, but we've got them on the radar.

Original comment by kak@google.com on 15 Nov 2011 at 7:29

[GoogleCodeExporter]

"extract" is fine, too, forgot that in my list.

I just needed that method today.

Original comment by j...@nwsnet.de on 15 Nov 2011 at 8:16

[GoogleCodeExporter]

Original comment by fry@google.com on 16 Nov 2011 at 7:39

• Removed labels: Milestone-Release11

[GoogleCodeExporter]

What about just Optionals.collect()?

Original comment by kevinb@google.com on 21 Nov 2011 at 10:50

[GoogleCodeExporter]

Yes.

Original comment by j...@nwsnet.de on 22 Nov 2011 at 9:59

[GoogleCodeExporter]

Optionals.collect() works for me.

Original comment by wasserman.louis on 30 Nov 2011 at 9:04

[GoogleCodeExporter]

As hinted by fry in issue #812, there already is `Optional.presentInstances` in 
trunk [1] that seems to supply this very functionality. Is it going to be moved 
and renamed?

[1] 
http://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/comm
on/base/Optional.java#166

Original comment by j...@nwsnet.de on 6 Dec 2011 at 10:46

[GoogleCodeExporter]

It's not going to be moved/renamed...we decided to go w/ a static method on 
Optional instead of another class (Optionals).

The reasoning was that since Optional isn't an interface (and it's non-user 
extendable), it makes more sense to just stick it on the class itself rather 
than add an entirely new class.

Original comment by kak@google.com on 6 Dec 2011 at 2:22

• Changed title: Add Optional.presentInstances
• Changed state: Fixed
• Added labels: Milestone-Release11

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:15

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:09

• Added labels: Restrict-AddIssueComment-Commit