Vavr: View as immutable java.util collections without copying

[seanf]

For efficient interop with general Java code, it would be very handy if the collections could be viewed as immutable java.util collections, without having to populate (eg) a new java.util.HashMap.

Fixed with #1952:

• [x] List.asJavaList()

Addressed in #1933:

• [ ] Map.asJavaMap()
• [ ] MultiMap.asJavaMap()
• [ ] Set.asJavaSet()

[danieldietrich]

Great idea!!

[seanf]

There's some similar code in Dexx Collections which may or may not be useful: https://github.com/andrewoma/dexx/tree/master/collection/src/main/java/com/github/andrewoma/dexx/collection/internal/adapter

[paplorinc]

What should happen with the illegal methods, e.g. remove when they're backed by an immutable collection?

[danieldietrich]

#ThrowAllTheThings™
We could update the internal delegate reference, i.e. the view will be mutable...
Then it isn't a View anymore (in the classical sense).
Could functional Lenses/Prisms help?

[paplorinc]

Theoretically we could, but wouldn't it be very counter-intuitive, e.g.

final java.util.List<T> list = source.toJavaList();
java.util.Collections.shuffle(list);

would either throw new UnsupportedOperationException or would shuffle a linked list in O(N^2) time

[seanf]

I was just thinking the standard throw new UnsupportedOperationException approach, as taken by the immutable wrappers in the JDK[1], Guava immutable collections[2] and Scala[3]. It's not pretty, but it is documented behaviour for the Java collections - "Throws: UnsupportedOperationException - if the put operation is not supported by this map"[4].

If someone wants to get a mutable Java Map out of a Javaslang Map, that's what toJavaMap() is for - in most cases mutations would probably far more efficient with a standard java.util.HashMap than a tricksy delegating implementation. Except in cases where the initial copy is expensive and there are few mutations afterwards. But I think that would be a less common use case.

[1] https://docs.oracle.com/javase/8/docs/api/java/util/Collections.html#unmodifiableMap-java.util.Map-
[2] https://google.github.io/guava/releases/snapshot/api/docs/com/google/common/collect/ImmutableList.html#add-int-E-
[3] http://docs.scala-lang.org/overviews/collections/conversions-between-java-and-scala-collections.html
[4] https://docs.oracle.com/javase/8/docs/api/java/util/Map.html#put-K-V-

[paplorinc]

If someone wants to get a mutable Java Map out of a Javaslang Map, that's what toJavaMap()

But if we give them a wrapper, they won't get a mutable Map

[seanf]

I'm not suggesting a change to the existing method toJavaMap(), just to add a new method.

• asJavaMap() -> view as immutable java.util.Map
• toJavaMap() -> copy contents to java.util.HashMap

[seanf]

But for a future Javaslang API, I suppose it would be possible to remove the toJava* methods if desired. The standard Java collections can easily be constructed (by copying) from another collection, like this:

java.util.Map<Foo> myMutableMap = new java.util.HashMap<>(myJavaslangMap.asJavaMap());

But it might be better to have both methods, to make it more obvious that the view is immutable.

Now I'm thinking asJavaMap() should be called asImmutableJavaMap(). That should make it obvious.

[paplorinc]

FYI: @danieldietrich, @seanf: https://github.com/javaslang/javaslang/pull/1736

[danieldietrich]

As always we align to Scala. Scala has JavaConverters. See also this.

scala> import scala.collection.JavaConverters._
import scala.collection.JavaConverters._

scala> Vector(1, 2, 3).asJava
res0: java.util.List[Int] = [1, 2, 3]

Naming

For Javaslang this means that we have one new method, named asJava.

Type hierarchy

We really want to have a Traversable.asJava() that returns java.util.Collection<T>. But this isn't possible in Java. Especially java.util.Map does not extends Collection. Therefore we can't define asJava() at the top level (Traversable). Instead we have these methods:

interface Seq<T> extends Traversable<T> {
    java.util.List<T> asJava();
}

interface IndexedSeq<T> extends Seq<T> {
    @Override
    default java.util.List<T> asJava() {
        return TODO("random access specific view");
    }
}

interface LinearSeq<T> extends Seq<T> {
    @Override
    default java.util.List<T> asJava() {
        return TODO("linear access specific view");
    }
}

interface Set<T> extends Traversable<T> {
    default java.util.Set<T> asJava() {
        return TODO("set view");
    }
}

interface Map<K, V> extends Traversable<Tuple2<K, V>> {
    default java.util.Map<K, V> asJava() {
        return TODO("map view");
    }
}

interface Multimap<K, V> extends Traversable<Tuple2<K, V>> {
    @Override
    default java.util.Map<K, Collection<V>> asJava() {
        return TODO("map view");
    }
}

Notes:

• For now we do no conversion for PriorityQueue or Tree. For sure, we could convert to sth. (like java.util.List) just for the sake to _convert it to sth_. But for now it is not needed.
• In Javaslang we do not have Iterator.asJava(). Iterator already extends java.util.Iterator. Additionally, as of 3.0.0, Iterator will not extends Iterable any more (see also #1295).

Update:

Scala has only converters for _mutable_ collections:

scala> Vector(1, 2, 3).asJava
<console>:14: error: value asJava is not a member of scala.collection.immutable.Vector[Int]
       Vector(1, 2, 3).asJava

We take another approach. We throw UnsupportedOperationException in the case of mutators like add(), put() and remove().

I provided an update for the above. Sorry for providing you with moving targets. I need to loop while providing impls, read: 1) play around, 2) get an impression, 3) update the constraints, 4) goto 1)

[danieldietrich]

This one needs definitely a section in our documentation. I will do that.

[danieldietrich]

@paplorinc Also I would love to see a packag-private javaslang.collection.JavaConverters helper class that contains static helpers that are called from within our collections.

Example:

public interface IndexedSeq<T> extends Seq<T> {
    @Override
    default java.util.List<T> asJava() {
        return JavaConverters.asJava(this);
    }
}

class JavaConverters {
    static <T> List<T> asJava(IndexedSeq<T> source) {
        return TODO("random access specific view");
    }
}

(@paplorinc I fixed a typo: toJava -> asJava)

[paplorinc]

@danieldietrich, please check my solution at https://github.com/javaslang/javaslang/pull/1736/commits/28d5c60eb3139d361163858c957ec27acecd2f6b

[danieldietrich]

@paplorinc my parents in law visit us - will do it this evening...

[danieldietrich]

Information on next iteration

• Throwing is no good idea, it will lead to endless suffer. We should implement the mutating methods.
• It is a good practice to define a clear interface (_namely the asJava() methods_) rather than exposing our internal view implementation types.
• We need to implement appropriate equals, hashCode and toString methods. The toString should output the same as ArrayList.
• We should also implement Serializable
• Please always fully qualify java collection types in our Javaslang codebase, even if there is actually no name clash. (Fully qualifying java.util.functions is not necessary.)

class JavaConverters {

    private JavaConverters() {
    }

    static <T> java.util.List<T> asJava(IndexedSeq<T> seq) {
        return new IndexedSeqAsJavaList<>(seq);
    }

    static <T> java.util.List<T> asJava(LinearSeq<T> seq) {
        return new LinearSeqAsJavaList<>(seq);
    }

    static <T> java.util.Set<T> asJava(Set<T> set) {
        return new SetAsJavaSet<>(seq);
    }

    static <K, V> java.util.Map<K, V> asJava(Map<K, V> map) {
        return new MapAsJavaMap<>(map);
    }

    static <K, V> java.util.Map<K, Collection<V>> asJava(Multimap<K, V> map) {
        return new MultimapAsJavaMap<>(map);
    }

    // -- private view implementations

    private static abstract class SeqAsJavaList<T> implements java.util.List<T> {

        protected abstract Seq<Object> delegate();

        // override only methods of List that share the same implementation for IndexedSeq and LinearSeq!
        @Override
        public int size() {
            return delegate().size();
        }

        @Override
        public boolean isEmpty() {
            return delegate().isEmpty();
        }

        @Override
        public boolean contains(Object o) {
            return delegate().contains(o);
        }

        // et cetera

        @Override
        public boolean equals(Object o) {

//            if (o == this) {
//                return true;
//            } else if (o instanceof java.util.List) {
//                return Collections.areEqual(delegate(), (java.util.List<?>) o);
//            } else {
//                return false;
//            }

            // java collections are not equal if references refer to the same memory location
            return o == this;
        }

        @Override
        public int hashCode() {
            // return delegate().hashCode();
            return super.hashCode();
        }

        @Override
        public String toString() {
            // not tested, should output the same as java.util.ArrayList
            return delegate().mkString("[", ",", "]");
        }

    }

    private static class IndexedSeqAsJavaList<T> extends SeqAsJavaList<T> implements Serializable {

        private static final long serialVersionUID = 1L;

        private IndexedSeq<Object> delegate;

        IndexedSeqAsJavaList(IndexedSeq<T> delegate) {
            this.delegate = IndexedSeq.narrow(delegate);
        }

        @Override
        protected IndexedSeq<Object> delegate() {
            return delegate;
        }

        // override the unimplemented methods
        // ...

        // no need to throw
        @Override
        public boolean add(T element) {
            delegate = delegate.append(element);
            return true;
        }
    }    
}

[paplorinc]

@danieldietrich, it's difficult to code in the browser, do you think it would be easier if you would merge the PR and change those instead?

[seanf]

Throwing is no good idea, it will lead to endless suffer. We should implement the mutating methods.

There might be a use case for that too, but there's a reason people use Guava's ImmutableList etc, despite the impurity of "optional" mutation methods. Sometimes you just want it to be an immutable list. OTOH if you want a mutable list, you should start with one of the many mutable list implementations, not an immutable one. (Or call toJavaList() to make a mutable copy.)

Scala does throw if you convert an immutable collection and then call a mutator method: http://docs.scala-lang.org/overviews/collections/conversions-between-java-and-scala-collections.html

[danieldietrich]

@seanf

but there's a reason people use Guava's ImmutableList etc

In Guava there is no other way around throwing, they just _wrap_ a mutable List. If they would allow to modify it they effectively would have ... a mutable List.

The one bad thing with throwing RuntimeException is that it happens at _runtime_. No one can beware us of ending up with an application with _undefined_ behavior (which is the worst thing that can happen).

Scala did not choose _the right path_ in any case regarding their collection library. That's also a reason for an effort to completely rethink the collection library.

These are the pros and cons I see by making the view _writable_:

Pros:

• The implementation is _safe(!)_
  
  
  
  • No 'broken' programs in terms of throwing RuntimeException
  
  
  • No problems with variance / heap pollution like described in our Cats & Dogs blog post because the underlying delegate is a persistent collection
  
  
• Especially we can be sure that the original collection (our delegate) is not modified
• We widen the use cases. If a user needs to use the java collection interfaces he may create a modifiable view in O(1). That's blazing fast!
• If the user needs unmodifiable versions he may call one of the unmodifiable*() helpers in j.u.Collections. The other way round is not possible: We can't simply make an unmodifiable collection modifiable.

Cons:

• Not really a con: The application does not throw, if the user calls a mutator method but did not want to call it.

Summed up I see only pros. I think some Java libs did only go the way of creating immutable collection wrappers that throw in order to secure their apps regarding MUTABLE-12. MUTABLE-2 is the only way around this security hole without copying the _mutable_ collection. But our underlying delegate is _persistent_. This security hole does not exist for us.

Which is your biggest fear / what can go wrong? Or is it only "but the other guys do it the other way"?

[paplorinc]

The one bad thing with throwing RuntimeException is that it happens at runtime.

:+1:

If the user needs unmodifiable versions he may call one of the unmodifiable*() helpers in j.u.Collections.

:+1:, haven't thought of that!

Summed up I see only pros.

Let me give a few additional pros and cons to make your list more balanced :)

Pro:

• The user expects a mutable java List, as that's what he's used to. By trying to access the last element via the listiterator (which is possible in an ArrayList and the doubly linked LinkedList), he will be surprised that it's linear - minor issue in my opinion.
• using it the view for a small Vector as if it were mutable will result in linear append - still a minor issue
• the advantages of persistence (i.e. that mutations don't propagate globally) is not preserved

Pro:

• Users can convert to and from the java world (we may want to optimize for these cases when users want to convert back to immutable views) basically for free
• Immutable collections can be disguised as mutable ones when giving it out to the world

[paplorinc]

Which is your biggest fear / what can go wrong? Or is it only "but the other guys do it the other way"?

Proud of you @danieldietrich, I like it when we dare to be pioneers!

[seanf]

I'm just pointing to Guava because I see an important use case for immutable java.util.Lists, and so did Google. (Some of their reasoning here: https://github.com/google/guava/wiki/ImmutableCollectionsExplained)

I guess I also think creating another (much less efficient) form of mutable list is a lot of effort (and perhaps maintenance complexity) for very limited value. I like immutable objects. Being able to say this java.util.List is "the same" as that javaslang List is very useful. Once you introduce mutability, that might not be true for long. Just because you can implement the optional mutation methods doesn't mean you must, or should.

I assume javaslang's Iterator.remove() throws UnsupportedOperationException? It's ugly, but sometimes ugly APIs force that on you.

For me as a new user, I use javaslang collections for their immutability. (I've been looking for a good, well-documented persistent collection in Maven Central for quite a while.) If I need to view such a javaslang collection as a Java collection, it's for interop, not because I wish I had created a mutable collection.

If you implement a mutable java.util.List view of a javaslang immutable List, will you also be creating a javaslang.MutableList (view) for completeness? The java.util interfaces have a few warts, and they don't have all the nice javaslang methods.

I'm not saying there's no use case for mutable views of immutable collections. In a way, they ~remind me of Clojure's refs, and~ seem like a clever idea. But maybe they aren't a core feature for a library which (to me) is about immutable collections, and could instead live outside the javaslang core.

Yes, Collections.unmodifiable* is an option. I think it would be friendlier if the immutable view were available directly from the javaslang collection methods though. (Especially since I think the immutable collection is the truer representation of the javaslang collection, although I'm apparently alone in that view. The possibility of unwanted exceptions is just an unfortunate consequence of the optional methods in the Java Collections API.)

Note that adding an unmodifiable wrapper may make it difficult or impossible to emulate Scala's ability to give you the same Java List every time you convert a Scala/Javaslang list, or to do round-trip conversions. http://docs.scala-lang.org/overviews/collections/conversions-between-java-and-scala-collections.html

In any case, if you're willing to break from Scala precedents where it makes sense, I think trying to make a Seq look like a List (despite the big differences in performance guarantees for some operations) is a Scala mistake which should not be emulated.

[danieldietrich]

I'm just pointing to Guava because I see an important use case for immutable java.util.Lists, and so did Google. (Some of their reasoning here: https://github.com/google/guava/wiki/ImmutableCollectionsExplained)

Guava says: Safe for use by untrusted libraries.
I think: How can throwing a RuntimeException be safe!?

Guava says: Thread safe: can be used by many threads with no risk of race conditions.
I think: Same here

Guava says: Doesn't need to support mutation, and can make time and space savings with that assumption. All immutable collection implementations are more memory-efficient than their mutable siblings (analysis)
I think: The memory footprint is equivalent (plus a constant for the wrapper) to that of the persistent collection.

Guava says: Can be used as a constant, with the expectation that it will remain fixed
I think: An _unsafe_ constant that throws if it is touched at the wrong place. Not good, I will not build my code on such 'things'.

I guess I also think creating another (much less efficient) form of mutable list is a lot of effort (and perhaps maintenance complexity) for very limited value. I like immutable objects. Being able to say this java.util.List is "the same" as that javaslang List is very useful. Once you introduce mutability, that might not be true for long. Just because you can implement the optional mutation methods doesn't mean you must, or should.

• I see no maintenance hell here: flat hierarchy, few methods, mostly delegating - easy!
• "much less efficient" -> tradeoff between fast creation (= view) and most efficient ops (= converted), user can decide

I assume javaslang's Iterator.remove() throws UnsupportedOperationException? It's ugly, but sometimes ugly APIs force that on you.

• I thought about implementing it in a second step, if easily(!) possible (did I mention it?)

For me as a new user, I use javaslang collections for their immutability. (I've been looking for a good, well-documented persistent collection in Maven Central for quite a while.) If I need to view such a javaslang collection as a Java collection, it's for interop, not because I wish I had created a mutable collection.

Persistence is much more than immutability. Writing operations is possible but new versions are created. For me a writeable List presentation is more natural.

But yes, indeed, the security hole already mentioned (heap pollution) does also exist for our mutable List view. But that's expected by users because... it is a mutable List!

If you implement a mutable java.util.List view of a javaslang immutable List, will you also be creating a javaslang.MutableList (view) for completeness? The java.util interfaces have a few warts, and they don't have all the nice javaslang methods.

No, definitely not. I was asked that several times and I always answered that Java already has mutable collections and the Stream extension for additional operations. I see no use-case for creating more mutable collections. The effort of creation and maintenance will be overkill compared to the gain.

I'm not saying there's no use case for mutable views of immutable collections. In a way, they remind me of Clojure's refs, and seem like a clever idea. But maybe they aren't a core feature for a library which (to me) is about immutable collections, and could instead live outside the javaslang core.

(see above / persistence vs immutability)

Yes, Collections.unmodifiable* is an option. I think it would be friendlier if the immutable view were available directly from the javaslang collection methods though. (Especially since I think the immutable collection is the truer representation of the javaslang collection, although I'm apparently alone in that view. The possibility of unwanted exceptions is just an unfortunate consequence of the optional methods in the Java Collections API.)

:+1:

Note that adding an unmodifiable wrapper may make it difficult or impossible to emulate Scala's ability to give you the same Java List every time you convert a Scala/Javaslang list, or to do round-trip conversions. http://docs.scala-lang.org/overviews/collections/conversions-between-java-and-scala-collections.html

No, we achieve the same. The conversion in both directions will be O(1). (That's the essence they talk about.)

In any case, if you're willing to break from Scala precedents where it makes sense, I think trying to make a Seq look like a List (despite the big differences in performance guarantees for some operations) is a Scala mistake which should not be emulated.

There are single linked Lists, lazy linked lists, random access Lists, etc. Java has only random access Lists. Converting IndexedSeq to Java List seems to be more straight-forward... But we will do it also for Seqs because it works out of the box.

[danieldietrich]

@seanf but maybe you are right... let me think a while about it... Javaslang should not be the next Cats & Dogs library.

[danieldietrich]

@seanf @paplorinc I thought about the whole topic.

My strategy with Javaslang collections is to provide a safe, more concise and more powerful alternative to the old-school, mutable Java standard collections.

The following is considered to be a safe programming style:

List<T> f(List<U> input) {
    // 1. we don't modify input
    // 2. we might use mutable local variables that do not escape to the outside
    // 3. we return an immutable output
}

In general we follow this programming style as implementor of f. But also we expect this when we are caller of f. It seems to be a good idea to let asJava() return immutable objects. Given that we enforce (by throwing) that no mutator methods are called when passing our objects to other functions.

But in the end, the developers are responsible to produce stable code with a defined/reproducable behavior.I see use-cases where it might be of interest to have mutable views:

Returning a mutable list is ok, when created in a local context:

List<T> localContext() {
    Seq<T> seq = ...;
    ...
    // users of our API will not mutate the result for security reasons
    // if a mutable List is needed we might use toList() instead
    return seq.asList();
}

When having conversions Seq-List-Seq, a mutable List might come handy:

Seq<T> localContext(Seq<U> seq) {
    j.u.List<T> list = seq.asJava(); // produce a List view on seq in O(1)
    mutateTheSeqBehindTheCurtain(list); // performs operations with a time/mem complexity of seq
    return Seq.ofAll(list) // recognizes the view and pulls out the current delegate in O(1)
        .map(...);
}

We might provide a special API that makes the mutable context local:

Seq<T> localContext(Seq<U> seq) {
    // conversion back and forth in O(1)
    return seq.asJava(list -> {
            mutateTheSeqBehindTheCurtain(list);
    }).map(...);
}

But how often is this use-case really needed?

I now aggree to @seanf that returning immutable objects might be the better variant.

[emmanueltouzery]

just to add my grain of salt: IMO the option to easily convert to mutable java collections is crucial. We've been able to convert a relatively big codebase to (mostly) javaslang because we knew we can port an isolated unit to generate and operate on javaslang collections, then get a java collection from it trivially and pass it on to the rest of the program. We ported the app piece by piece in that fashion. If by default asJavaList() returns an immutable collection that throws when you attempt to modify it, then that cannot be used for that purpose, because we can't assume anything about what the legacy code we're communicating with is doing. So for such a porting, I think you really need a functionality such as the existing toJavaList(). If it gets verbose to achieve that, we (our team) would have had to write helpers for conversion, and that's just friction getting in the way of migrating to javaslang.

[danieldietrich]

@paplorinc Additionally we should provide j.u.SortedMap, j.u.SortedSet conversion.

Accordingly we need to change method declarations within our type hierarchy:

// Ok
Seq: abstract j.u.List asJava()
IndexedSeq: default j.u.List asJava()
LinearSeq: default j.u.List asJava()

// TODO
Set: default j.u.Set asJava()
SortedSet: @Override default j.u.SortedSet asJava()

// TODO
Map: default j.u.Map asJava()
Map: @Override default j.u.SortedMap asJava()

toString() looks like this:

        SortedMap<Integer, String> map = new TreeMap<>();
        map.put(1, "a");
        map.put(2, "b");
        println(map);

// output:
{1=a, 2=b}

        SortedSet<Integer> set = new TreeSet<>();
        set.add(1);
        set.add(2);
        println(set);

// output:
[1, 2]

[danieldietrich]

@paplorinc In fact we should convert sorted variants to j.u.NavigableSet and j.u.NavigableMap. But for now we can't do that because there is an open issue to add this functionality to Javaslang: #1317 (I'm on it).

For now we return SortedSet and SortedMap. I will created another issue (see #1747) for the navigable interfaces that depends on #1317

[danieldietrich]

@emmanueltouzery Thank you for your feedback. The conversion methods toJava*() will stay with us. What we need to know is if it would help in a real-life application to have a way to additionally create mutable views.

// Takes new mem by creating a new ArrayList and copying all elements in O(n).
// As a result, operations on that List are the same as those of ArrayList.
toJavaList()

// Additional method. creates a List in O(1) by providing a wrapper around a persistent collection.
// As a result, operations on that List are the same as those of the underlying persistent collection.
asJava()

What do you think? Would you propagate a mutable view to the outside or copy a collection instead?

/cc @paplorinc @seanf

[paplorinc]

@danieldietrich, we should create real-world tests to see how both feel and decide later.
I'm ok with both, though I like your new approach more now that I've tried it (i.e. the mutable, backed by immutable).

[emmanueltouzery]

For me as I said, I think the toJava* are needed, glad they're not in question. For the new asJava views, no opinion. I'm sure there are useful use-cases, didn't need it so far though. They definitely sound cool (although allowing modifications sounds overkill, but maybe it can be implemented without the complexity exploding).

[seanf]

On 6 December 2016 at 22:46, Daniel Dietrich [e-mail]
wrote:

@emmanueltouzery https://github.com/emmanueltouzery Thank you for your
feedback. The conversion methods toJava*() will stay with us. What we
need to know is if it would help in a real-life application to have a way
to additionally create mutable views.

// Takes new mem by creating a new ArrayList and copying all elements in O(n).// As a result, operations on that List are the same as those of ArrayList.
toJavaList()
// Additional method. creates a List in O(1) by providing a wrapper around a persistent collection.// As a result, operations on that List are the same as those of the underlying persistent collection.
asJava()

I think it's very important that the method names make a clear distinction
between the mutable version and the immutable version, to minimise
surprises.

In fact, at the risk of taking things too far (or too far off track), I
wonder if it's a good idea to add an ImmutableList *interface *which would
look a bit like Guava's ImmutableList class:
https://google.github.io/guava/releases/snapshot/api/docs/com/google/common/collect/ImmutableList.html
Basically It would just extend java.util.List, but with deprecation
warnings on the mutation methods.

(In my own experience with Guava, I sometimes use deliberate references to
Guava's ImmutableList, to help keep straight mutable versus immutable
lists. It's certainly not a guarantee because an ImmutableList is still a
List, but it does help a little, just as a form of documentation.)

If the user just assigns the result to a java.util.List variable, this
interface will be useless, naturally. But if the object is assigned to a
variable of type javaslang.jcollection.ImmutableList (which IDEs would
offer to do automatically in various situations, eg "create local
variable"), this may help to remind the user that this is a special
immutable list, and that mutator methods should be avoided.

What do you think? Would you propagate a mutable view to the outside or

copy a collection instead?

So far I've just been using toJava* when mutation was required, and it was
perfectly fine in that particular situation.

But are you asking whether the existing method toJava* should switch from
copying, to providing a mutable view? Given the performance differences, I
don't think the *existing *method should make such a change.

I suppose someone should ask the obvious next question. Does it make sense
to provide three options, or is it too much bloat?

1. toJavaList - O(n): copies to a java.util.ArrayList
2. asJavaList/asMutableJavaList - O(1): provides a mutable view by
   wrapping a mutable pointer to a javaslang List
3. asImmutableJavaList - O(1): provides an immutable view[1]

[1] Preferably by wrapping a javaslang List, *not *by wrapping an
unmodifiableList around a mutable wrapper around a javaslang List.

/cc @paplorinc https://github.com/paplorinc @seanf
https://github.com/seanf

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/javaslang/javaslang/issues/1728#issuecomment-265141479,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADTi7cBggSRYuCIZ-eDR6tVVHf0lji9ks5rFVlCgaJpZM4K_0Ve
.

[danieldietrich]

I see valid use-cases for all variants. We should

• add asJava() that returns mutable j.u. collections
• add asJava(Consumer<List<T>> that allows list mutation in a local scope. The mutated seq is finally returned
• add asJavaImmutable() that returns an immutable view

Update: Let's note provide a asJava(Cosumer) for now. It is too much effort to override it on every subtype (which is needed because we need to adjust the return type of the method...).

Details (example: IndexedSeq):

interface IndexedSeq<T> extends Seq<T> {

    @Override
    default List<T> asJava() {
        return JavaConverters.asJava(this, false);
    }

    @Override
    default List<T> asJavaImmutable() {
        return JavaConverters.asJava(this, true);
    }

}

The view impl contains a flag imutable that states, if the collection can me mutated or not:

private static class SeqAsJavaList<T> implements java.util.List<T>, Serializable {

        private static final long serialVersionUID = 1L;

        private Seq<T> delegate;
        private final boolean immutable;

        SeqAsJavaList(Seq<T> delegate, boolean immutable) {
            this.delegate = delegate;
            this.immutable = immutable;
        }

        @Override
        public boolean add(T element) {
            ensureMutable();
            delegate = delegate.append(element);
            return true;
        }

        // ...

        private void ensureMutable() {
            if (immutable) {
                throw new UnsupportedOperationException();
            }
        }

}

Update: Could be also a private static helper:

final class JavaConverters {
        ...
        private static void ensureMutable(boolean immutable) {
            if (immutable) {
                throw new UnsupportedOperationException();
            }
        }
        ...
}

/cc @paplorinc

[danieldietrich]

@paplorinc or you do it more JIT friendly, like in Vector:

@Override
public boolean add(T element) {
    if (mutable) {
        delegate = delegate.append(element);
        return true;
    } else {
        throw new UnsupportedOperationException();
    }
}

_Note: naming the instance var 'mutable' makes more sense then_

[paplorinc]

I agree that both alternatives have value (mutable and throwing), but combining them makes this more than simple delegation, which I find worst than any simple alternative.

[danieldietrich]

Depends on #1800 because ListView.addAll(empty) needs to return false, i.e. _not modified_.

[danieldietrich]

Note: First I thought it is a good idea to mimic Java's j.u.List behavior. But that applies only to the mutable case. The immutable impls are inconsistent.

Simple example: set(index, T) and remove(index) (but there are more...)

[nfekete]

I vote for the immutable version, with the API method called asJavaImmutable() or asImmutableJava(). This way the API consumer should be very well aware what gets, even without reading the javadocs for that method. That is, he/she will get an immutable view of the collection through the standard Java Collections API. He/She shouldn't try to mutate it and shouldn't pass it to some other black box code which would try to mutate it (which would we a bad practice anyway from that black box code). One might say that throwing a runtime exception is unexpected behavior happening at runtime. I think that isn't the case (i.e. it's not unexpected), considering the above, but even then, it should be quite obvious with minimal amount of testing. I think Javaslang shouldn't try to guard against every possible misuse of the library, it's enough to aim for a well defined behavior, which IMO is satisfied with the above.

[danieldietrich]

@nfekete Agreed, we shouldn't obfuscate things and raise the possibility of errors by choosing the wrong naming. I liked the short asJava() but I'm totally ok with naming the methods asJavaMutable() and asJavaImmutable().

[danieldietrich]

The Java Collection API docs:

The "destructive" methods contained in this interface, that is, the methods that modify the collection on which they operate, are specified to throw UnsupportedOperationException if this collection does not support the operation. If this is the case, these methods may, but are not required to, throw an UnsupportedOperationException if the invocation would have no effect on the collection. For example, invoking the addAll(Collection) method on an unmodifiable collection may, but is not required to, throw the exception if the collection to be added is empty.

In order to keep things (and code!) as simple as possible, our immutable views will throw an UnsupportedOperationException _before_ performing non-null and index checks. Half of the standard immutable java.util.List implementations do it this way, the other half performs checks first.

[nfekete]

Yes, I think throwing unconditionally is better, because accepting an empty collection by addAll() could let people believe the receiver collection is actually mutable. Calling any kind of mutating method on an immutable collection is an error by itself.

[danieldietrich]

@nfekete

I think Javaslang shouldn't try to guard against every possible misuse of the library, it's enough to aim for a well defined behavior, which IMO is satisfied with the above.

your views are thoroughly thought through and very helpful - good to have you here!

(I'm currently finishing the ListView and finally chose the names 'asJava()' and 'asJavaImmutable()')

[danieldietrich]

We layed the grind stone here. Remaining views will be implemented in #1933