Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: > create Streamable and ParallelStreamable interface and use them in Collection > and Optional > _Mailing list message from [Alan Snyder](mailto:javali...@cbfiddle.com) on > [core-libs-dev](mailto:core-libs-...@mail.openjdk.java.net):_ > > Ah, if only one could define a type alias Streamable = > Supplier>... If you use that, you'd lose some semantics that the method name would have given you. I feel that @liach 's solution is cleaner, since that preserves this method name. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: > create Streamable and ParallelStreamable interface and use them in Collection > and Optional I believe that the accompanying JBS issue (JDK-8272137) can be closed. Maybe a mention of how the discussion in this PR was resolved (with code examples) could be added there before closing it. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable [plug for Extensible interface]
This provides an opportunity for me to promote what I believe is a much more
important missing interface, namely, an interface that supports a semantic
replacement for type casting.
Using type casting (instanceof) is a really bad way to test an object for an
optional capability.
The reason is that instanceof is strictly limited by how the object is
implemented. It will only work if the object *directly* implements the
interface. It does not support an object that might provide the requested
interface using an auxiliary object. It doesn’t support delegation at all. If
you try to wrap an object with a transparent wrapper implemented using
delegation, the wrapper must support exactly the interfaces that you expect
from the wrapped object. If some of those are optional, you wind up with many
versions of the wrapper to ensure that instanceof will work on the wrapper as
expected.
This is hardly a new idea. I’ve seen this idea in several major libraries. But,
because Java does define its own version of this interface, this approach
cannot be used in general.
I suspect it would be useful for some of the problems being discussed here.
For concreteness, this is how I define it:
public interface Extensible
{
@Nullable T getExtension(@NotNull Class c);
}
with a static method used in place of instanceof:
public static @Nullable T getExtension(@Nullable Object o, @NotNull
Class c)
{
if (o == null) {
return null;
}
if (c.isInstance(o)) {
return c.cast(o);
}
if (o instanceof Extensible) {
Extensible x = (Extensible) o;
return x.getExtension(c);
}
return null;
}
> On Aug 17, 2021, at 10:54 AM, CC007
> wrote:
>
> On Mon, 9 Aug 2021 12:28:23 GMT, CC007
> wrote:
>
>> create Streamable and ParallelStreamable interface and use them in
>> Collection and Optional
>
> Ah ok, I see your point. In the case that you want to have something be only
> `Streamable`, you can create an interface like this (fixed missing method
> type param and added `ofCollection`:
>
> public interface Streamable {
>
>Stream stream();
>
>static Streamable ofIterable(Iterable iterable) {
>return () -> StreamSupport.stream(iterable.spliterator(), false);
>}
>
>static Streamable ofCollection(Collection collection) {
>return collection::stream;
>}
> }
>
> This will indeed allow you to only expose the `stream()` method, even to the
> degree that you can't even expose the other methods with type casting, which
> is a nice side effect. You could also add a static method for `ofOptional`,
> if required, but you made a good point about `Optional.stream`'s general use
> case (though it could still be used as a stream when needed).
>
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007
wrote:
> create Streamable and ParallelStreamable interface and use them in Collection
> and Optional
Ah ok, I see your point. In the case that you want to have something be only
`Streamable`, you can create an interface like this (fixed missing method type
param and added `ofCollection`:
public interface Streamable {
Stream stream();
static Streamable ofIterable(Iterable iterable) {
return () -> StreamSupport.stream(iterable.spliterator(), false);
}
static Streamable ofCollection(Collection collection) {
return collection::stream;
}
}
This will indeed allow you to only expose the `stream()` method, even to the
degree that you can't even expose the other methods with type casting, which is
a nice side effect. You could also add a static method for `ofOptional`, if
required, but you made a good point about `Optional.stream`'s general use case
(though it could still be used as a stream when needed).
At first I didn't fully understand how that would resolve the issue for
whenever you need something that is both `Iterable` and `Streamable`, but after
rereading your comment I came up with the following interface:
/**
* This is an interface that specifies that its content can be traversed and
acted upon,
* either through iteration or using streams
*/
public interface Traversable extends Streamable, Iterable {
static Traversable ofIterable(Iterable iterable) {
return new Traversable() {
@Override
public Stream stream() {
return StreamSupport.stream(iterable.spliterator(), false);
}
@Override
public Iterator iterator() {
return iterable.iterator();
}
};
}
static Traversable ofStreamable(Streamable streamable) {
return new Traversable() {
@Override
public Stream stream() {
return streamable.stream();
}
@Override
public Iterator iterator() {
return streamable.stream().iterator();
}
};
}
static Traversable ofCollection(Collection collection) {
return new Traversable() {
@Override
public Stream stream() {
return collection.stream();
}
@Override
public Iterator iterator() {
return collection.iterator();
}
};
}
}
For anyone who's doubtful about the arguments against this change, you can find
this code in use in demo2 in my demo repo:
https://github.com/CC007/InterfaceSegregationDemo/tree/master/src/main/java/com/github/cc007/interfacesegregationdemo/demo2
-
PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007
wrote:
> create Streamable and ParallelStreamable interface and use them in Collection
> and Optional
With modern java, you can always create a `Streamable` on your own (and have
specifications/documentations that a simple `Supplier` lacks) and implement it
like:
public interface Streamable {
Stream stream();
static Streamable ofIterable(Iterable iterable) {
return () -> StreamSupport.stream(iterable.spliterator(), false);
}
}
The lack of such an interface in the JDK is not really a problem.
> Are you suggesting that there are deeper architectural issues with the
> hierarchy-heavy collection stack?
I am saying that your sample project has unnecessarily many interfaces for your
collection model, where many of them have little virtue on their own. This
interface mash is susceptible to accidentally introducing unwanted features
into the hierarchy and having method or specification clashes for
implementation.
Looking back on identifying efficiently streamable objects, people can usually
find efficient streams by the method return types. Your interface doesn't allow
people to identify `BufferedReader.lines()` as an efficiently streamable
target, for instance.
In addition, I think the main purpose of `Optional.stream()` is to allow using
it in stream flatmapping than to promote stream creation with optionals.
-
PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: > create Streamable and ParallelStreamable interface and use them in Collection > and Optional The List and Collection interface was almost directly taken from java.util (apart from the small feature interfaces, that I extracted from them), so it would make sense that those classes use an older design. Are you suggesting that there are deeper architectural issues with the hierarchy-heavy collection stack? Would it be better if a list contained a collection, rather than inheriting from it? If that is the case, anything that is proposed in this PR is merely aiming at symptoms of a possibility much larger underlying problem, which might require going back to the drawing board for all collection related features. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: > create Streamable and ParallelStreamable interface and use them in Collection > and Optional You can view the mailing lists at https://mail.openjdk.java.net/mailman/listinfo and subscribe there. If you want to send a mail, just send one to say `jdk-dev` at `openjdk.java.net` (and carbon copy if you reply to a specific person so the list can see a copy) On a separate note, @CC007 your model falls back to the fairly-old inheritance vs embodiment (extending a class vs using it as a field) debate. Nowadays, I think Java has evolved to reduce excessive reliances on inheritances, as there can be clashes (for example, a class cannot implement a comparator for two different type arguments). Also due to these potential of clashing, in a lot of java code, interface implementation classes avoid implementing multiple interfaces at once. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: > create Streamable and ParallelStreamable interface and use them in Collection > and Optional Also, sorry for my Millennial lack of knowledge of older communication methods, but if I wanted to reply to a specific thread in a mailing list, how would I do that? Does it require certain content in the subject and/or quoted text in the message body? It does feel way less intuitive than just clicking the comment box or clicking `... -> Quote reply` on a comment in this PR - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 16 Aug 2021 03:39:02 GMT, Tagir F. Valeev wrote: >> create Streamable and ParallelStreamable interface and use them in >> Collection and Optional > > Mostly agreed with Brian. Judging from 7 years of using Stream API, I can say > that this abstraction would not solve any real problem. If you need a way to > create many identical streams on demand, just accept `Supplier>`. > This allows more flexibility for clients. They can not only supply > `myCollection::stream` or `myOptional::stream` but also `() -> > Arrays.stream(myArray)`, `() -> IntStream.range(...).boxed()`, `() -> > myCollection.stream().filter(something)` or whatever else. A dedicated > `Streamable` interface is too limited and will require adapters in many cases > but you can already adapt anything to `Supplier>`. People already > use `Supplier>` idiom pretty often, so creating a new `Streamable` > interface would add an API mess: some people would stick with `Supplier` and > others would migrate to `Streamable`. So I vote to reject this PR. > > I said "mostly" because I think that PR is a good starting point for > discussion. It's much easier to explain which enhancement you are proposing > if you already present some code. And we are already at corelibs-dev, as PR > comments are mirrored there, and for some people, it's more comfortable to > discuss via GitHub interface, as you don't have to subscribe and get tons of > unrelated e-mails, you can concentrate on a single discussion only. So in my > opinion, it's completely ok to write code and create a PR before the > discussion, even if it's likely to be thrown away. It is fine that the pull request is closed for now, but I agree with @amaembo that if this conversation is indeed mirrored there, then it shouldn't be a problem to discuss the proposal here, along with the proposed changes. I actually worked out a usecase (See: https://github.com/CC007/InterfaceSegregationDemo), where I partially reimagined the Collection interface stack and its neighboring interfaces/classes. I also added a `Demo` class to show how I imagined to use these interfaces and to show how the granularity of these interfaces can be useful. The fact that I have to create copies of interfaces and delegates for implementations makes me feel like something is lacking in the collection stack and that for this usecase `Supplier` wouldn't cut it, but feel free to fork and refactor my code to prove me wrong. PS: while creating the Demo class, I noticed that it couldn't be analyzed if the return value of a method with type ` & Addable & Iterable & RandomAccess>` could be safely cast to a new local variable with type ` & Addable` and that you can't declare variables of type ` & Addable` (it won't let you use the `&` when using the `?`), but I digress. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Sun, 15 Aug 2021 01:43:32 GMT, Brian Goetz wrote: > To reiterate: These issues were explored in the JSR 335 EG and it was agreed > that this abstraction did not carry its weight. Yes, we explored this when the Stream API was being designed. It's hard, if not impossible, to capture all decisions for posterity. In the passing years I don't think anything significantly new has arisen for us to revisit that decision. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Sun, 15 Aug 2021 02:45:17 GMT, CC007 wrote: >> create Streamable and ParallelStreamable interface and use them in >> Collection and Optional > > I read through [these > posts](http://mail.openjdk.java.net/pipermail/lambda-libs-spec-experts/2013-June/thread.html#1910) > and while I did see good reasons for not moving stream to Iterable, I didn't > see any mention of a separate Streamable-like interface, so could you > elaborate on that it "did not carry its weight"? @CC007 Please follow the course of action that @briangoetz has requested. This needs to be discussed on the appropriate mailing list (core-libs-dev in this case) prior to submitting a pull request. It is premature to review this PR. I invite you to refer to the [OpenJDK Developers' Guide](https://openjdk.java.net/guide/), specifically the [Socialize your change](https://openjdk.java.net/guide/#socialize-your-change) section. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
Ah, if only one could define a type alias Streamable = Supplier>... > On Aug 15, 2021, at 8:42 PM, Tagir F.Valeev wrote: > > On Mon, 9 Aug 2021 12:28:23 GMT, CC007 > wrote: > >> create Streamable and ParallelStreamable interface and use them in >> Collection and Optional > > Mostly agreed with Brian. Judging from 7 years of using Stream API, I can say > that this abstraction would not solve any real problem. If you need a way to > create many identical streams on demand, just accept `Supplier>`. > This allows more flexibility for clients. They can not only supply > `myCollection::stream` or `myOptional::stream` but also `() -> > Arrays.stream(myArray)`, `() -> IntStream.range(...).boxed()`, `() -> > myCollection.stream().filter(something)` or whatever else. A dedicated > `Streamable` interface is too limited and will require adapters in many cases > but you can already adapt anything to `Supplier>`. People already > use `Supplier>` idiom pretty often, so creating a new `Streamable` > interface would add an API mess: some people would stick with `Supplier` and > others would migrate to `Streamable`. So I vote to reject this PR. > > I said "mostly" because I think that PR is a good starting point for > discussion. It's much easier to explain which enhancement you are proposing > if you already present some code. And we are already at corelibs-dev, as PR > comments are mirrored there, and for some people, it's more comfortable to > discuss via GitHub interface, as you don't have to subscribe and get tons of > unrelated e-mails, you can concentrate on a single discussion only. So in my > opinion, it's completely ok to write code and create a PR before the > discussion, even if it's likely to be thrown away. > > - > > PR: https://git.openjdk.java.net/jdk/pull/5050 >
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: > create Streamable and ParallelStreamable interface and use them in Collection > and Optional Mostly agreed with Brian. Judging from 7 years of using Stream API, I can say that this abstraction would not solve any real problem. If you need a way to create many identical streams on demand, just accept `Supplier>`. This allows more flexibility for clients. They can not only supply `myCollection::stream` or `myOptional::stream` but also `() -> Arrays.stream(myArray)`, `() -> IntStream.range(...).boxed()`, `() -> myCollection.stream().filter(something)` or whatever else. A dedicated `Streamable` interface is too limited and will require adapters in many cases but you can already adapt anything to `Supplier>`. People already use `Supplier>` idiom pretty often, so creating a new `Streamable` interface would add an API mess: some people would stick with `Supplier` and others would migrate to `Streamable`. So I vote to reject this PR. I said "mostly" because I think that PR is a good starting point for discussion. It's much easier to explain which enhancement you are proposing if you already present some code. And we are already at corelibs-dev, as PR comments are mirrored there, and for some people, it's more comfortable to discuss via GitHub interface, as you don't have to subscribe and get tons of unrelated e-mails, you can concentrate on a single discussion only. So in my opinion, it's completely ok to write code and create a PR before the discussion, even if it's likely to be thrown away. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On 15/08/2021 14:29, Brian Goetz wrote: For the third time: This discussion illustrates why the PR was premature; the design was not agreed upon first. High-level design discussions (i.e., "is this a good design", "is this a good idea at all", "are we solving the right problem", "does it need to be solved in the JDK") should happen first on the discussion lists; if they happen on a PR like this, that's clear evidence that important steps have been skipped. The OpenJDK developers guide has been getting some TLC in recent months. One of the improvements is the "Socialize your change" section [1] which does try to make it clear that API proposals should be discussed on the mailing list before going near a PR. -Alan [1] https://openjdk.java.net/guide/#socialize-your-change
Re: RFR: 8272137: Make Collection and Optional classes streamable
For the third time: This discussion illustrates why the PR was premature; the design was not agreed upon first. High-level design discussions (i.e., "is this a good design", "is this a good idea at all", "are we solving the right problem", "does it need to be solved in the JDK") should happen first on the discussion lists; if they happen on a PR like this, that's clear evidence that important steps have been skipped. On 8/14/2021 10:48 PM, CC007 wrote: On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: create Streamable and ParallelStreamable interface and use them in Collection and Optional I read through [these posts](http://mail.openjdk.java.net/pipermail/lambda-libs-spec-experts/2013-June/thread.html#1910) and while I did see good reasons for not moving stream to Iterable, I didn't see any mention of a separate Streamable-like interface, so could you elaborate on that it "did not carry its weight"? - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: > create Streamable and ParallelStreamable interface and use them in Collection > and Optional I read through [these posts](http://mail.openjdk.java.net/pipermail/lambda-libs-spec-experts/2013-June/thread.html#1910) and while I did see good reasons for not moving stream to Iterable, I didn't see any mention of a separate Streamable-like interface, so could you elaborate on that it "did not carry its weight"? - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: > create Streamable and ParallelStreamable interface and use them in Collection > and Optional To reiterate: These issues were explored in the JSR 335 EG and it was agreed that this abstraction did not carry its weight. In any case, it is premature to bring a PR for a significant API change that has not been discussed first on corelibs-dev. Please withdraw the PR, and if you want to continue the discussion, bring it to corelibs-dev. Sent from my iPad > On Aug 14, 2021, at 9:10 PM, CC007 ***@***.***> wrote: > > > I understand what you are proposing. I do not believe Streamable carries its > weight. > > That argument seems subjective. Could you elaborate on that in an objective > manner? > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub, or unsubscribe. > Triage notifications on the go with GitHub Mobile for iOS or Android. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Sun, 15 Aug 2021 01:07:10 GMT, Brian Goetz wrote: > I understand what you are proposing. I do not believe Streamable carries its > weight. That argument seems subjective. Could you elaborate on that in an objective manner? - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: > create Streamable and ParallelStreamable interface and use them in Collection > and Optional I understand what you are proposing. I do not believe Streamable carries its weight. Sent from my iPad > On Aug 14, 2021, at 8:53 PM, CC007 ***@***.***> wrote: > > > I object to this change. These issues were explored in the JSR 335 EG and it > was agreed that this abstraction did not carry its weight. In any case, it is > premature to bring a PR for a significant API change that has not been > discussed first on corelibs-dev. Please withdraw the PR, and if you want to > continue the discussion, bring it to corelibs-dev. > > I agree with the spliterator performance issues and the ability to be able to > create IntStreams from Iterable. Therefore option 1 and 2 from JDK-8272137 > seem fairly unfeasible to implement. This was already hinted at there, but > for different reasons. > > This PR however implements option 4: an option that doesn't actually make the > Iterable class have a stream method, but instead abstracts the stream method > to an interface. This method is then implemented by Collection and Optional. > It also allows other code to implement this interface. > > Using this implementation, you don't have the issues that the JSR 335 EG > specified that would be present if Iterable itself were to be made > Streamable. You wouldn't want that anyway, because streamability and > iterability are two similar but very separate concerns in Java. > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub, or unsubscribe. > Triage notifications on the go with GitHub Mobile for iOS or Android. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Mon, 9 Aug 2021 12:28:23 GMT, CC007 wrote: > create Streamable and ParallelStreamable interface and use them in Collection > and Optional Changed the title, since it seemed to cause confusion. - PR: https://git.openjdk.java.net/jdk/pull/5050
Re: RFR: 8272137: Make Collection and Optional classes streamable
On Sat, 14 Aug 2021 15:13:39 GMT, Brian Goetz wrote: > I object to this change. These issues were explored in the JSR 335 EG and it > was agreed that this abstraction did not carry its weight. In any case, it is > premature to bring a PR for a significant API change that has not been > discussed first on corelibs-dev. Please withdraw the PR, and if you want to > continue the discussion, bring it to corelibs-dev. I agree with the spliterator performance issues and the ability to be able to create IntStreams from Iterable. Therefore option 1 and 2 from JDK-8272137 seem fairly unfeasible to implement. This was already hinted at there, but for different reasons. This PR however implements option 4: an option that doesn't actually make the Iterable class have a stream method, but instead abstracts the stream method to an interface. This method is then implemented by Collection and Optional. It also allows other code to implement this interface. Using this implementation, you don't have the issues that the JSR 335 EG specified that would be present if Iterable itself were to be made Streamable. You wouldn't want that anyway, because streamability and iterability are two similar but very separate concerns in Java. - PR: https://git.openjdk.java.net/jdk/pull/5050
