Loose ends: Optional

[Brian Goetz]

> I think we have clearly two options, and I'm fine with both of them:
> Option A: We want to add map/flatMap.
>    In that case, we also need to add a way to do the conversion back and
> fore to null (orNull() and fromNullable()).

I see your point, but I don't think this necessarily follows (though it 
is certainly something to discuss.)

Neither Guava, Fugue, nor j.u.Optional allow Optional to hold a null.

The fromNullable proposal is merely a convenience method; anyone can 
write it as an expression (o == null ? O.empty() : O.of(o)) or as a 
static method (MyFoo.ofNullable).  So it is by no means *necessary*.

Now, the same can be superficially said of map/flatMap -- people can 
easily write these themselves too -- but there is a key difference; 
fluency.  People want flatMap not because it offers functionality people 
can't write for themselves, but that they can't otherwise do it fluently 
at the end of a chain.  But at the other end of the chain, whether the 
factory method is "Optional.ofNullable" or "MyFoo.ofNullable", there is 
effectively no difference, so these are different cases.

So, I would amend your statement to: if we add map/flatMap, we then have 
to ask the question of whether we should also add ofNullable.

I find this method also in the category of "mostly harmless", but left 
it out of the initial proposal in spirit of minimality; limiting the set 
of proposed methods to those that where having them in the JDK type adds 
the most value.

If the EG will only take filter/map/flatMap if we add ofNullable, I'm OK 
with that.  (On the other hand, if where you are going is "if we add X 
we have to add Y, and then we have to add Z, and X+Y+Z is too much, so 
let's do none of them", then I'm not so OK.)

> Note that the original code proposed by Brian below already does a
 > null check in map().

There are two null checks.  First is a check if the mapper function 
itself is null -- since a null function is clearly a bug.  I assume you 
are talking of the other, which is the "treat null return as nothing 
there" interpretation of the mapper result. There's a valid choice here 
-- NPE vs interpret as empty.  The latter is consistent with the other 
methods we've added to Map, such as computeIfAbsent.  But we can also 
discuss throwing NPE here.

> Regardless the option chosen above,
>    - Optional should not implement Iterable or have a forEach method. It
> makes the code weird to read.

Sam made a very compelling argument for forEach based on experience at 
Twitter over lunch last week.  I'll summarize, but Sam please correct me 
if I mangle it.  Basically: in their library development efforts, there 
were a lot of "unrelated" containerish classes (including a variant of 
Optional), which each had their own class-specific method names.  Over 
time, the method names converged to a common vocabulary (filter, map, 
forEach) even though they had no common supertype, and as a result, the 
perceived usability went way up.

The current ifPresent(Consumer) is something I pulled out of a hat and 
the name does not connect to anything.  On the other hand, 
forEach(Consumer) is familiar from a lot of other classes.  I prefer 
forEach, on the theory of not introducing new names for new stuff.  (I 
already hate the forEach / forEachRemaining distinction between 
Collection and Iterator, and don't want to make this problem any worse.)

The "don't implement Iterable" and "don't call it forEach" all feel like 
they are motivated by "make it less useful so people won't be tempted to 
use it as much."  There's a (possible) value in that Optional; giving 
users multiple reasonable ways to get it at means they have to distort 
their code less to use Optional effectively.

> In that case, flatMap is in my opinion useless because the code that returns null will not magically
>   be re-written to send an Optional as parameter.

I think you may have a misunderstanding of what flatMap does?  The point 
here is that you *already* have a method that returns an Optional, and 
you want to avoid double-wrapping with Optional<Optional<T>>, which is 
what you'd get if you did optional.map().  For example, for "find the 
first element in stream B that is larger than any element in stream A":

   Optional<T> aMax = a.max();
   Optional<T> nextInB = aMax.flatMap(m -> b.filter(e -> e > m)
                                            .findFirst());

(written non-fluently for explicitness, but in reality you'd likely 
chain off a.max()).  Without flatMap, it would be messier.

>    - filter doesn't seem to pull it's own weight, for what it worth, I
> never use it on an Option in Scala.

Agreed this is probably the least useful of the bunch, but again, can't 
be easily added with fluency by users, the semantics and implementation 
are obvious, and its absence might look weird given few things support 
map but not filter.

So, here are the decisions I see from this list:

- Given map/flatMap, must we / do we also need ofNullable and orNull?
- If the mapper returns null, should we continue to silently convert to 
empty optional or throw NPE?
- Does renaming ifPresent to forEach help or hurt?  What about 
implementing Iterable?