Skip to content

API Design Review: Java 8 Stream Naming Conventions #678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
benjchristensen opened this issue Dec 24, 2013 · 14 comments
Closed

API Design Review: Java 8 Stream Naming Conventions #678

benjchristensen opened this issue Dec 24, 2013 · 14 comments
Milestone

Comments

@benjchristensen
Copy link
Member

Let's compare against the Java 8 Streams API and try to match as closely to it as possible. We can add aliases where it makes sense and possibly deprecate and migrate to their names if they are better.

The idea is to support Java developers as they use the Stream API and then want to move to async and be as similar as possible.

It should try and achieve the "Principle of Least Surprise" for developers.

Ideally someone could have code written for the Stream API and just swap out the Stream for an Observable and it "just work".

/cc @headinthebox and @jhusain
/cc @bondolo (from Oracle) if you have any thoughts on this

@akarnokd
Copy link
Member

Unless one uses call-site lambdas, "just swapping" won't work due base functional type mismatch:

stream.filter(v -> v % 2 == 0)
observable.filter(v -> v % 2 == 0)
// but
Predicate<Integer> p = v -> v % 2 == 0
stream.filter(p);
observable.filter(p); // type mismatch
observable.filter(p::test);

Some renames/matches of functionality

Stream API Observable API
filter where
mapToInt primitive observable ???
sorted N/A
peek ???
limit take(n)
forEach doOnEach
toArray toBlockingObservable().toList().toArray()
anyMatch exists().toBlockingObservable().single()
count count().toBlockingObservable().single()
allMatch all().toBlockingObservable().single()
noneMatch all(not()).toBlockingObservable().single()
findFirst materialize().toBlockingObservable().first()
findAny N/A, would be identical to findFirst

Some Stream methods exit the stream, this is why there are that many toBlockingObservable calls in the table.

@sirinath
Copy link

Keep the .Net naming and usage intact much as possible. Any language specific idioms can core exit with this.

This would help towards less confusion and easy portability in case you maintain a .net API also.

This can be extended to the other language bindings also. E.g. idiomatic Scala API should ideally not hide the .Net of F# usage (as in this case F# being closer to Scala / Clojure than C#) but co exist with the idiomatic language version.

@jhusain
Copy link

jhusain commented Dec 27, 2013

I sympathize with the plight of the cross-platform RX user - I am one. However I strongly disagree with the idea of using non-idiomatic names just to make porting code between platforms easier. We tried that with RxJS, and it caused no end of confusion. There are many RX users who use RX across platforms. However there is a much larger potential audience that does not yet use RX, will very likely be exposed to Streams, and will approach reactive programming for the first time in the next two years.

J

Dictated using voice recognition. Please forgive the typos.

On Dec 27, 2013, at 6:29 AM, Suminda Dharmasena [email protected] wrote:

Keep the .Net naming and usage intact much as possible. Any language specific idioms can core exit with this.

This would help towards less confusion and easy portability in case you maintain a .net API also.

This can be extended to the other language bindings also. E.g. idiomatic Scala API should ideally not hide the .Net of F# usage (as in this case F# being closer to Scala / Clojure than C#)


Reply to this email directly or view it on GitHub.

@sirinath
Copy link

What I say is use idiomatic names wherever possible but leave the more familiar naming and usage also. The idiomatic changes would be a few so there will be little overhead.

@sirinath
Copy link

Best is that if there is a possibility / feasibility coordinate across the Rx implementations and the .net project is also open source so there is coherence and API convergence.

@benjchristensen
Copy link
Member Author

These are the only ones that seem worth considering:

Stream API Observable API
filter where
limit take(n)
forEach doOnEach

We have already done filter. I am okay with aliasing take and limit.

The forEach name may also be good. The question then is whether it's a synonym with doOnEach or subscribe(Action1<T>). I think it's the later, as someone shouldn't need to forEach(f).subscribe().

Should we alias subscribe with forEach?

void forEach(Action1<? super T>)
void forEach(Action1<? super T>, Action1<Throwable>)
void forEach(Action1<? super T>, Action1<Throwable>, Action0)

@daschl
Copy link
Contributor

daschl commented May 20, 2014

@benjchristensen I agree with the latter, especially people coming from java 8 streams, it's not obvious that in Rx you then have to subscribe in addition.

@benjchristensen benjchristensen added this to the 0.19 milestone May 20, 2014
@headinthebox
Copy link
Contributor

OK with forEach, I think RxJs has that as well.

@benjchristensen
Copy link
Member Author

Let's move ahead with this.

benjchristensen added a commit to benjchristensen/RxJava that referenced this issue May 20, 2014
- as per Java 8 Stream naming conventions in discussion ReactiveX#678
benjchristensen added a commit to benjchristensen/RxJava that referenced this issue May 20, 2014
As per discussion at ReactiveX#678

Code like this is now supported:

```java
Observable.from(1, 2, 3).forEach(System.out::println);
Observable.from(1, 2, 3).toBlocking().forEach(System.out::println);
```
@benjchristensen
Copy link
Member Author

Completed in #1232

@benjchristensen
Copy link
Member Author

    public static void main(String[] args) {
        Observable.from(1, 2, 3).forEach(System.out::println);
        Observable.from(1, 2, 3).toBlocking().forEach(System.out::println);

        Observable.from(1, 2, 3).limit(2).forEach(System.out::println);
    }

@jhusain
Copy link

jhusain commented May 20, 2014

Note that in JavaScript this method removes the data from the monad.

That said, we should definitely match the stream APIs closely as possible for Java 8.

Dictated using voice recognition. Please forgive the typos.

On May 20, 2014, at 9:14 AM, headinthebox [email protected] wrote:

OK with forEach, I think RxJs has that as well.


Reply to this email directly or view it on GitHub.

@benjchristensen
Copy link
Member Author

We have both the non-blocking and blocking forms of forEach now. The blocking one removes it from the monad.

Observable.from(1, 2, 3).forEach(System.out::println);
Observable.from(1, 2, 3).toBlocking().forEach(System.out::println);

@headinthebox
Copy link
Contributor

Makes sense.

zsxwing referenced this issue in zsxwing/RxJava Jun 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants