Re: [External] : Re: Collection::getAny discussion

[forax]

- Mail original -
> De: "Stuart Marks" 
> À: "Remi Forax" 
> Cc: "core-libs-dev" 
> Envoyé: Mercredi 12 Mai 2021 03:03:33
> Objet: Re: [External] : Re: Collection::getAny discussion

> On 5/10/21 3:31 AM, Remi Forax wrote:
>> Thinking a little more about conflating "first" and "any".
>> I wonder if we have not already cross the Rubicon on that matter,
>> 
>> If we have a HashSet or any collections and using Stream.findFirst()
>>var collection = new HashSet<>(...);
>>var result = collection.stream().findFirst().orElseThrow();
>> 
>> We will get the result of collection.iterator().next()
>> 
>> So adding a default method getFirst() on Collection that returns the result 
>> of
>> iterator().next() is pretty coherent, no ?
> 
> Not really. Streams have a runtime notion of being ORDERED, and no static 
> type.
> Adding Collection.getFirst() has no similar runtime notion.

The characteristics of a Spliterator/Stream reflects the properties of the 
collection from which the Spliterator was derived.

Anyway, the spec of Stream.findFirst() is not described in term of being 
ORDERED or not, it just says returns the first element of a Stream and a Stream 
can be created on any collections, ordered or not.
So having a method getFirst() on Collection returning the first element of the 
collection makes sense to me, the same way getting the first element of a 
Stream makes sense,
even if it implies getting a random element in case of a Set.

> This proposal is to add a static type for ordering/reversibility and 
> corresponding operations for it.

Given that it's a proposal, i think it's fair to discuss other options if most 
of the intent of your proposal or the one from Tagir is kept.  

> 
> I'd still like to hear about the use cases for getAny or whatever we want to
> call
> the thing. Are callers interested in the collection having zero-or-one, 
> exactly
> one,
> zero or more, or one or more elements? Talking about iterator().next() without
> considering the use cases, and their implications for pattern matching, is
> short-sighted.

Pattern matching (AFAIK) can not be used outside of a switch/instanceof, if as 
a user i'm required to wrap getFirst() inside a switch in order to get the 
first element, i will find that API cumbersome.

> 
> s'marks

Rémi

Re: [External] : Re: Collection::getAny discussion

[Stuart Marks]

On 5/10/21 3:31 AM, Remi Forax wrote:

Thinking a little more about conflating "first" and "any".
I wonder if we have not already cross the Rubicon on that matter,

If we have a HashSet or any collections and using Stream.findFirst()
   var collection = new HashSet<>(...);
   var result = collection.stream().findFirst().orElseThrow();

We will get the result of collection.iterator().next()

So adding a default method getFirst() on Collection that returns the result of 
iterator().next() is pretty coherent, no ?


Not really. Streams have a runtime notion of being ORDERED, and no static type. 
Adding Collection.getFirst() has no similar runtime notion. This proposal is to add 
a static type for ordering/reversibility and corresponding operations for it.


I'd still like to hear about the use cases for getAny or whatever we want to call 
the thing. Are callers interested in the collection having zero-or-one, exactly one, 
zero or more, or one or more elements? Talking about iterator().next() without 
considering the use cases, and their implications for pattern matching, is 
short-sighted.


s'marks

Re: Collection::getAny discussion

[Remi Forax]

- Mail original -
> De: "Stephen Colebourne" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 30 Avril 2021 23:15:45
> Objet: Re: Collection::getAny discussion

> On Fri, 30 Apr 2021 at 19:50, Stuart Marks  wrote:
>> You're asking for something that's somewhat different, which you called the
>> "find
>> the first element when there is only one" problem. Here, there's a 
>> precondition
>> that
>> the collection have a single element. (It's not clear to me what should 
>> happen
>> if
>> the collection has zero or more than one element.)
> 
> I think any get() or getAny() method on Collection is semantically
> equivalent to iterator.next(). I'm not sure there is another viable
> option.

Thinking a little more about conflating "first" and "any".
I wonder if we have not already cross the Rubicon on that matter,

If we have a HashSet or any collections and using Stream.findFirst()
  var collection = new HashSet<>(...); 
  var result = collection.stream().findFirst().orElseThrow();

We will get the result of collection.iterator().next()

So adding a default method getFirst() on Collection that returns the result of 
iterator().next() is pretty coherent, no ?

[...]

> 
> Stephen

Rémi

Re: Collection::getAny discussion

[Donald Raab]

To clarify, RichIterable is not a subclass of Collection. As we discovered in 
JDK 15, a problem exists when we add default methods to interfaces that might 
get “mixed” with other interfaces that already have those methods. There are a 
few potential issues with adding zero argument default methods to common 
interfaces. The two easiest to reason about that we have experience with are:

1. Signatures don’t match (e.g. getAny() returns Optional) - very bad - lots of 
pain caused - forces backwards incompatible change to library APIs - this 
happened when default sort() was added to List and returned void
2. Signatures match, but no concrete implementations when mixing competing 
default implementations - work for library developers - forces clients to 
upgrade to new version of library to use new version of JDK (e.g. EC 10.4 
upgrade to work with JDK 15 for CharSequence.isEmpty())

https://stuartmarks.wordpress.com/2020/09/22/incompatibilities-with-jdk-15-charsequence-isempty/
 

I think adding getAny() to Collection makes sense, that’s why we have defined 
the method on RichIterable. For Eclipse Collections, option #2 is the only 
option that works (getAny() returns E). It will cause us OSS library 
maintainers a bunch of work and a release upgrade. Knowing the direction and 
testing with early access versions (as we do) prior to our next release will 
help so we can start coding out any necessary concrete implementations wherever 
they don’t exist in the hierarchy.

The default getAny() implementation on RichIterable is different than just 
calling iterator.next() though. The getAny() method is currently defined as 
calling getFirst() which will return null in the case an iterable is empty. 

So this creates a new kind of potential issue we haven’t experienced where we 
could wind up with two getAny() methods with the same signature but with 
different specifications and results on empty. This will be potentially 
confusing for Eclipse Collections users as they could get different behavior 
from any JDK collections for getAny(), or libraries they use could get 
different behavior for Eclipse Collections types that extend JDK types. Do we 
change the Eclipse Collections specification to match the new JDK 
specification? Will this break anyone that currently uses it by causing new 
unexpected Runtime exceptions? I honestly don’t know.
 
For additional reference, there is also a getOnly() method on RichIterable 
which behaves differently than getAny(). 

https://www.eclipse.org/collections/javadoc/10.4.0/org/eclipse/collections/api/RichIterable.html#getOnly()

There are also getFirst() and getLast() methods on RichIterable which were 
deprecated in 6.0 but will never be removed. They were added to OrderedIterable 
where they make more sense.

 

> On Apr 30, 2021, at 4:14 PM, Brian Goetz  wrote:
> 
> While I agree that we should be careful, let's not paint ourselves into an 
> either/or strawman.  The choice is not "never add anything to Collection" vs 
> "let's dump every silly idea that comes to anyone's mind into Collection"; it 
> is, as always, going to involve tradeoffs between stability and evolution.  
> 
> We cannot constrain ourselves so hard that we cannot evolve the core 
> libraries because it might collide with someone else's subclass.  That's not 
> reasonable, nor is that good for Java.
> 
> On the other hand, we must continue to exercise care in many dimensions when 
> adding to libraries that are widely used and implemented -- which we already 
> do (so much so, in fact, that people are often frustrated by our seeming 
> conservatism.)  
> 
> 
> 
> 
> 
> 
> 
> On 4/30/2021 4:02 PM, Donald Raab wrote:
>> There is a default method getAny defined on the RichIterable interface in 
>> Eclipse Collections. Adding a getAny with the same signature to Collection 
>> is bound to cause a break similar to CharSequence.isEmpty did with JDK 15 
>> but possibly more extensive since RichIterable is the parent interface for 
>> all collection types in Eclipse Collections. Adding it with a different 
>> signature (returns Optional) could cause extensive damage.
>> 
>> https://www.eclipse.org/collections/javadoc/10.4.0/org/eclipse/collections/api/RichIterable.html#getAny()
>>  
>> 
>>  
>> 
>> I highly recommend we stop looking to add new zero-argument default methods 
>> to 20+ year Collection interfaces and hope that we don’t break anything. 
>> There seems to be desire to breathe life into the old Collection interfaces. 
>> IMO, we should just start planning and focusing on a Collections 2.0 design.
>> 
>> 
>>> On Apr 30, 2021, at 2:49 PM, Stuart Marks  
>>>  wrote:
>>> 
>>> Hi Henri,
>>> 
>>> I've changed the subject of this thread because I think it's out of scope 
>>> of the ReversibleCollection proposal. I don't mean to say that we can't 
>>> talk about it, but I 

Re: Collection::getAny discussion

[Stephen Colebourne]

On Fri, 30 Apr 2021 at 19:50, Stuart Marks  wrote:
> You're asking for something that's somewhat different, which you called the 
> "find
> the first element when there is only one" problem. Here, there's a 
> precondition that
> the collection have a single element. (It's not clear to me what should 
> happen if
> the collection has zero or more than one element.)

I think any get() or getAny() method on Collection is semantically
equivalent to iterator.next(). I'm not sure there is another viable
option.

>   * onlyElement -- if source has 1 element, returns it; throws NSEE if empty, 
> IAE if > 1
>   * toOptional -- if source has 0 or 1 elements, returns an Optional; 
> otherwise throws

These should be added to the JDK. They are useful.

Stephen

Re: Collection::getAny discussion

[Brian Goetz]

While I agree that we should be careful, let's not paint ourselves into 
an either/or strawman.  The choice is not "never add anything to 
Collection" vs "let's dump every silly idea that comes to anyone's mind 
into Collection"; it is, as always, going to involve tradeoffs between 
stability and evolution.


We cannot constrain ourselves so hard that we cannot evolve the core 
libraries because it might collide with someone else's subclass.  That's 
not reasonable, nor is that good for Java.


On the other hand, we must continue to exercise care in many dimensions 
when adding to libraries that are widely used and implemented -- which 
we already do (so much so, in fact, that people are often frustrated by 
our seeming conservatism.)








On 4/30/2021 4:02 PM, Donald Raab wrote:

There is a default method getAny defined on the RichIterable interface in 
Eclipse Collections. Adding a getAny with the same signature to Collection is 
bound to cause a break similar to CharSequence.isEmpty did with JDK 15 but 
possibly more extensive since RichIterable is the parent interface for all 
collection types in Eclipse Collections. Adding it with a different signature 
(returns Optional) could cause extensive damage.

https://www.eclipse.org/collections/javadoc/10.4.0/org/eclipse/collections/api/RichIterable.html#getAny()

I highly recommend we stop looking to add new zero-argument default methods to 
20+ year Collection interfaces and hope that we don’t break anything. There 
seems to be desire to breathe life into the old Collection interfaces. IMO, we 
should just start planning and focusing on a Collections 2.0 design.



On Apr 30, 2021, at 2:49 PM, Stuart Marks  wrote:

Hi Henri,

I've changed the subject of this thread because I think it's out of scope of the 
ReversibleCollection proposal. I don't mean to say that we can't talk about it, but I 
would like it to be decoupled from ReversibleCollection. I'm somewhat arbitrarily calling 
it "Collection::getAny" because something similar to that was mentioned by both 
Remi and Peter elsewhere in this thread. There is also a bunch of history in the bug 
database that contains related ideas.

Before we dive in, I want to explain why this is separate from ReversibleCollection. Most of the ideas, including yours, involve 
an implementation that does something like `iterator().next()`, in other words, getting the "first" element from an 
Iterator. Hey, there's getFirst() in ReversibleCollection, let's use that! No. The "first" element of an iterator is in 
general an arbitrary element, which is different from the "first" element in the structural ordering of elements 
provided by a ReversibleCollection. The "arbitrary" notion is captured by "getAny" so that's what I'll use as 
a working term. (Of course any actual API we might add would have a better name if we can find one.)

For a historical perspective, let's dig into the bug database and take a look 
at this bug:

https://bugs.openjdk.java.net/browse/JDK-4939317

This requests a method Collection.get(Object). This searches the collection for 
an element that equals() the argument and returns the element, or it returns 
null if the element isn't found. (Recall in those days it was impossible to add 
a method to an interface without breaking compatibility, so it also proposes 
various workarounds that are no longer necessary.)

There's a comment from Josh Bloch saying that Collection should have had a 
get() method as well as a no-arg remove() method. Well ok then! And he points 
to the then-new Queue interface that was delivered in Java SE 5. Queue adds the 
following methods that seem relevant to this discussion:

* E element() -- gets the head element, throws NSEE if empty
* E remove() -- removes and returns the head element, throws NSEE if empty

(It also adds peek() and poll(), which are similar to the above except they 
return null if empty.)

This is kind of odd, in that none of these methods satisfy what the bug's 
submitter was requesting, which is a one-arg get() method. Also, these methods 
are on Queue, which doesn't really help with collections in general.

You're asking for something that's somewhat different, which you called the "find 
the first element when there is only one" problem. Here, there's a precondition that 
the collection have a single element. (It's not clear to me what should happen if the 
collection has zero or more than one element.)

To throw a couple more variations into the mix, Guava has a couple Collectors 
(for streams) that do interesting things. The class is MoreCollectors:

https://guava.dev/releases/30.1.1-jre/api/docs/com/google/common/collect/MoreCollectors.html

and the collectors are:

* onlyElement -- if source has 1 element, returns it; throws NSEE if empty, IAE if 
> 1
* toOptional -- if source has 0 or 1 elements, returns an Optional; otherwise 
throws

These apply to streams, but I think you can see the applicability to Collection 
as well. In particular, your 

Re: Collection::getAny discussion

[Donald Raab]

There is a default method getAny defined on the RichIterable interface in 
Eclipse Collections. Adding a getAny with the same signature to Collection is 
bound to cause a break similar to CharSequence.isEmpty did with JDK 15 but 
possibly more extensive since RichIterable is the parent interface for all 
collection types in Eclipse Collections. Adding it with a different signature 
(returns Optional) could cause extensive damage.

https://www.eclipse.org/collections/javadoc/10.4.0/org/eclipse/collections/api/RichIterable.html#getAny()
 

I highly recommend we stop looking to add new zero-argument default methods to 
20+ year Collection interfaces and hope that we don’t break anything. There 
seems to be desire to breathe life into the old Collection interfaces. IMO, we 
should just start planning and focusing on a Collections 2.0 design.


> On Apr 30, 2021, at 2:49 PM, Stuart Marks  wrote:
> 
> Hi Henri,
> 
> I've changed the subject of this thread because I think it's out of scope of 
> the ReversibleCollection proposal. I don't mean to say that we can't talk 
> about it, but I would like it to be decoupled from ReversibleCollection. I'm 
> somewhat arbitrarily calling it "Collection::getAny" because something 
> similar to that was mentioned by both Remi and Peter elsewhere in this 
> thread. There is also a bunch of history in the bug database that contains 
> related ideas.
> 
> Before we dive in, I want to explain why this is separate from 
> ReversibleCollection. Most of the ideas, including yours, involve an 
> implementation that does something like `iterator().next()`, in other words, 
> getting the "first" element from an Iterator. Hey, there's getFirst() in 
> ReversibleCollection, let's use that! No. The "first" element of an iterator 
> is in general an arbitrary element, which is different from the "first" 
> element in the structural ordering of elements provided by a 
> ReversibleCollection. The "arbitrary" notion is captured by "getAny" so 
> that's what I'll use as a working term. (Of course any actual API we might 
> add would have a better name if we can find one.)
> 
> For a historical perspective, let's dig into the bug database and take a look 
> at this bug:
> 
> https://bugs.openjdk.java.net/browse/JDK-4939317
> 
> This requests a method Collection.get(Object). This searches the collection 
> for an element that equals() the argument and returns the element, or it 
> returns null if the element isn't found. (Recall in those days it was 
> impossible to add a method to an interface without breaking compatibility, so 
> it also proposes various workarounds that are no longer necessary.)
> 
> There's a comment from Josh Bloch saying that Collection should have had a 
> get() method as well as a no-arg remove() method. Well ok then! And he points 
> to the then-new Queue interface that was delivered in Java SE 5. Queue adds 
> the following methods that seem relevant to this discussion:
> 
> * E element() -- gets the head element, throws NSEE if empty
> * E remove() -- removes and returns the head element, throws NSEE if empty
> 
> (It also adds peek() and poll(), which are similar to the above except they 
> return null if empty.)
> 
> This is kind of odd, in that none of these methods satisfy what the bug's 
> submitter was requesting, which is a one-arg get() method. Also, these 
> methods are on Queue, which doesn't really help with collections in general.
> 
> You're asking for something that's somewhat different, which you called the 
> "find the first element when there is only one" problem. Here, there's a 
> precondition that the collection have a single element. (It's not clear to me 
> what should happen if the collection has zero or more than one element.)
> 
> To throw a couple more variations into the mix, Guava has a couple Collectors 
> (for streams) that do interesting things. The class is MoreCollectors:
> 
> https://guava.dev/releases/30.1.1-jre/api/docs/com/google/common/collect/MoreCollectors.html
> 
> and the collectors are:
> 
> * onlyElement -- if source has 1 element, returns it; throws NSEE if empty, 
> IAE if > 1
> * toOptional -- if source has 0 or 1 elements, returns an Optional; otherwise 
> throws
> 
> These apply to streams, but I think you can see the applicability to 
> Collection as well. In particular, your proposal is similar to what 
> onlyElement would look like if it were on Collection.
> 
> Let's summarize the variations so far:
> 
> * preconditions: exactly one element, at-most-one, at-least-one
> * behavior if preconditions not met: return null, return empty Optional, throw
>   exception
> * remove element or just peek
> * match a particular element, or return an arbitrary element
> 
> That's a lot of variations!
> 
> Before we talk about specific APIs, though, I wanted to talk about use cases. 
> Which of these variations are more useful or less useful? Which are likely to 
> appear in code? Henri gave a fairly specific example with a 

Collection::getAny discussion

[Stuart Marks]

Hi Henri,

I've changed the subject of this thread because I think it's out of scope of the 
ReversibleCollection proposal. I don't mean to say that we can't talk about it, but 
I would like it to be decoupled from ReversibleCollection. I'm somewhat arbitrarily 
calling it "Collection::getAny" because something similar to that was mentioned by 
both Remi and Peter elsewhere in this thread. There is also a bunch of history in 
the bug database that contains related ideas.


Before we dive in, I want to explain why this is separate from ReversibleCollection. 
Most of the ideas, including yours, involve an implementation that does something 
like `iterator().next()`, in other words, getting the "first" element from an 
Iterator. Hey, there's getFirst() in ReversibleCollection, let's use that! No. The 
"first" element of an iterator is in general an arbitrary element, which is 
different from the "first" element in the structural ordering of elements provided 
by a ReversibleCollection. The "arbitrary" notion is captured by "getAny" so that's 
what I'll use as a working term. (Of course any actual API we might add would have a 
better name if we can find one.)


For a historical perspective, let's dig into the bug database and take a look at 
this bug:


https://bugs.openjdk.java.net/browse/JDK-4939317

This requests a method Collection.get(Object). This searches the collection for an 
element that equals() the argument and returns the element, or it returns null if 
the element isn't found. (Recall in those days it was impossible to add a method to 
an interface without breaking compatibility, so it also proposes various workarounds 
that are no longer necessary.)


There's a comment from Josh Bloch saying that Collection should have had a get() 
method as well as a no-arg remove() method. Well ok then! And he points to the 
then-new Queue interface that was delivered in Java SE 5. Queue adds the following 
methods that seem relevant to this discussion:


 * E element() -- gets the head element, throws NSEE if empty
 * E remove() -- removes and returns the head element, throws NSEE if empty

(It also adds peek() and poll(), which are similar to the above except they return 
null if empty.)


This is kind of odd, in that none of these methods satisfy what the bug's submitter 
was requesting, which is a one-arg get() method. Also, these methods are on Queue, 
which doesn't really help with collections in general.


You're asking for something that's somewhat different, which you called the "find 
the first element when there is only one" problem. Here, there's a precondition that 
the collection have a single element. (It's not clear to me what should happen if 
the collection has zero or more than one element.)


To throw a couple more variations into the mix, Guava has a couple Collectors (for 
streams) that do interesting things. The class is MoreCollectors:


https://guava.dev/releases/30.1.1-jre/api/docs/com/google/common/collect/MoreCollectors.html

and the collectors are:

 * onlyElement -- if source has 1 element, returns it; throws NSEE if empty, IAE 
if > 1
 * toOptional -- if source has 0 or 1 elements, returns an Optional; otherwise 
throws

These apply to streams, but I think you can see the applicability to Collection as 
well. In particular, your proposal is similar to what onlyElement would look like if 
it were on Collection.


Let's summarize the variations so far:

 * preconditions: exactly one element, at-most-one, at-least-one
 * behavior if preconditions not met: return null, return empty Optional, throw
   exception
 * remove element or just peek
 * match a particular element, or return an arbitrary element

That's a lot of variations!

Before we talk about specific APIs, though, I wanted to talk about use cases. Which 
of these variations are more useful or less useful? Which are likely to appear in 
code? Henri gave a fairly specific example with a reasonable "success" case 
(preconditions met) but it's not clear what should happen if the preconditions 
aren't met. Clearly an API would have to choose. What would the use site look like? 
In particular, does the use site always have to check for null, or catch an 
exception, or something?


Answers to these questions will help determine what APIs, if any, we might want 
to add.

***

There's another thing looming in the distance, which is pattern matching. You might 
have seen one of Brian Goetz's talks on pattern matching futures. You can get a 
glimpse of some upcoming pattern matching features in JEP 405.


    http://openjdk.java.net/jeps/405

In particular, this JEP extends pattern matching with an /extraction/ step, where, 
if there is a match, record or array components can be extracted and bound to local 
variables. This is a step closer to /deconstruction patterns/, where arbitrary 
classes and interfaces (not just records or arrays) can participate in pattern 
matching. (See discussion of this at the end of the JEP.)