mkodekar / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Are there plans for an ImmutableOptionalList? #1795

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Since ImmutableLists don't accept nulls, are there any plans for something like:

    ImmutableOptionalList<T> extends ImmutableList<Optional<T>>

So that I can construct it with:

     ImmutableOptionalList.of("foo", null, "bar");

And get essentially the behavior as:

     ImmutableList.of(Optional.of("foo"), Optional.absent(), Optional.of("bar"));

I could write this myself, but only at a fairly higher storage cost than if 
Guava did it:  the cost of constructing an ImmutableList costs an array of n 
elements, but I also have to allocate the n Optional elements upfront, and 
those Optionals have to stick around for the lifetime of the ImmutableList.

A Guava ImmutableOptionalList could be more efficient than this: it could have 
the same permanent storage cost as an ImmutableList (i.e., only the array of n 
objects, some of which might be null), and the Optionals could be lazily 
created as elements were accessed, where an implementation might vaguely look 
like this:

    public T get(int index) {
      T t = this.delegate().get(index);
      return (t == null) ? Optional.of(t) : Optional.absent();
    }

(I would expect/hope that in practice, the allocation costs for these temporary 
objects would get optimized away at runtime by the JVM, especially in loops 
that iterate over the whole list?) 

If Guava did choose to supply this, I don't think that it would necessarily 
lead to requests for other Immutable/Optional combination classes, like an 
ImmutableOptionalSet or ImmutableMapWithOptionalValues.  (A Set version doesn't 
seem to make sense, since it would have at most one Optional.absent() 
element...and ImmutableMaps already have enough allocation per entry that the 
incremental cost of one Optional per value seems less significant.  

This request is only for ImmutableOptionalList, and only if the intuition is 
correct that it would be a roughly 50% space savings over the best a user can 
get by rolling their own ImmutableList<Optional>.

Apologies if this has already been discussed and/or is a duplicate request...

Original issue reported on code.google.com by Mickey.K...@gmail.com on 4 Jul 2014 at 2:54

GoogleCodeExporter commented 9 years ago
Ugh...re-reading the request, the ternary logic in the sample implementation of 
get() is clearly backwards, but hopefully the request still makes sense...  :-P 

Original comment by Mickey.K...@gmail.com on 4 Jul 2014 at 2:56

GoogleCodeExporter commented 9 years ago
This is actually a good request. Anyway, just out of curiosity, what should a 
ImmutableMapWithOptionalValues do for map.get(something) when something is not 
in the map? Null, Optional.absent(), or throw?

Original comment by JanecekP...@seznam.cz on 4 Jul 2014 at 5:52

GoogleCodeExporter commented 9 years ago
Hmm...I was just trying to do a strawman implementation and realized the 
problematic part:  if an ImmutableOptionalList doesn't maintain permanent 
references to the Optionals that wrap its elements, there's no way to return 
the same object on subsequent calls.  So you get this behavior:

   ImmutableList<Optional<String>> list = ImmutableOptionalList.of("foo", null, "bar");
   Optional<String> firstTime = list.get(0);
   Optional<String> secondTime = list.get(0);
   assert firstTime.equals(secondTime)  // true
   assert firstTime == secondTime // false  :-(

If ImmutableOptionalList did extend ImmutableList, then anyone who wrote code 
that cared about object identity, like this:

    void method(ImmutableList<E> list) {
      synchronized (list.get(0)) {
        ...
      }
    }

would be mighty surprised if anyone ever passed in an ImmutableOptionalList.  
In short, I *think* this would violate the part of ImmutableList that says:

    <p>Unlike {@link Collections#unmodifiableList}, which is a <i>view</i> of a
    separate collection that can still change, an instance of {@code
    ImmutableList} contains its own private data and will <i>never</i> change.

If I'm reading that correctly, that the view of an ImmutableList never changes, 
then ImmutableOptionalList could never extend ImmutableList (because it can't 
offer an view that never changes object identity).  If that's right, then I 
don't need Guava to solve this...I'm just as well off rolling my own, doing 
something like this:

    // This version is *mostly* immutable, and is more space efficient:
    public static <T> List<Optional<T>> optionalListFor(Iterable<? extends T> nullableReferences) {
      return Lists.transform(Lists.newArrayList(nullableReferences), Optionals#fromNullable);
    }

    // This version is *truly* immutable, but at a higher space cost compared to the *mostly* immutable one:
    public static <T> ImmutableList<Optional<T>> immutableOptionalListFor(Iterable<? extends T> nullableReferences) {
      return ImmutableList.copyOf(Lists.transform(nullableReferences, Optionals#fromNullable));
    }

As a general observation, the root cause of this request had to do with the 
immutable collections' relationship with null elements, as has been discussed 
ad nauseum in other places, like 
https://code.google.com/p/guava-libraries/wiki/ImmutableCollectionsExplained 
and 
https://code.google.com/p/guava-libraries/wiki/UsingAndAvoidingNullExplained .  
And as those pages point out, in cases where you don't have a natural 
NullElement, wrapping nullable elements in Optionals is one way of getting them 
to play nice together.  It certainly isn't Guava's fault that people keep 
finding themselves in this situation; it's just unfortunate for those people 
that using ImmutableLists with Optionals comes at a cost of both clunkier 
syntax for the author/reader at write time and more storage space at runtime.

I don't see a way to withdraw an issue in the googlecode UI, but let me at 
least say that I understand why this clunkiness isn't likely to be addressed, 
at least certainly not as I had proposed -- so feel free to close this issue 
for me.

Original comment by Mickey.K...@gmail.com on 5 Jul 2014 at 3:28

GoogleCodeExporter commented 9 years ago
@JanecekP -- I don't have a need for an ImmutableMapWithOptionalValues, but my 
gut reaction is that this is dictated by Map#get(), so that if the key is 
missing, the #get() contract says it should return null.

All three of your use cases seem reasonable (that you would want some method 
call that returns null or Optional.absent() or throws an Exception), but 
perhaps that object isn't a Map or that method call isn't Map#get().  Perhaps 
that object could be a Function, instead?  If so, then given a Map of 
potentially nullable keys/values, I would expect that you could use some 
combination of Maps#transformValues(), Functions#forMap(), and 
Functions#compose to produce your three cases.

Original comment by Mickey.K...@gmail.com on 5 Jul 2014 at 3:50

GoogleCodeExporter commented 9 years ago
I don't think that adding a whole new suite of APIs for this makes sense. Just 
wrap elements in Optional.of/fromNullable as needed.

Original comment by cgdecker@google.com on 7 Jul 2014 at 5:07

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

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

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

GoogleCodeExporter commented 9 years ago

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