Skip to content
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

Initial clean up for Combining Observables docs #6135

Merged
merged 2 commits into from
Aug 4, 2018
Merged

Initial clean up for Combining Observables docs #6135

merged 2 commits into from
Aug 4, 2018

Conversation

ahmedre
Copy link
Contributor

@ahmedre ahmedre commented Aug 4, 2018

This patch updates the style of the combining observables documentation to be similar to that of #6131 and adds examples for most of the operators therein. Refs #6132.

What's left for combining observables:

  • documentation for join, groupJoin, and rxjava-joins
  • add documentation for concat, concatEager, and withLatestFrom

This patch updates the style of the combining observables documentation
to be similar to that of #6131 and adds examples for most of the
operators therein. Refs #6132.

#### combineLatest Example

```java
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was unsure whether to go with an example like this one or something more abstract but potentially more "useful" like:

Observable<NewsItems> latestNews = getLatestNews();
Observable<Weather> latestWeather = getLatestWeather();
Observable.combineLatest(latestNews, latestWeather, (news, weather) -> Pair(news, weather))
      .subscribe(item -> renderUi(item.first, item.second));

and am also unsure how you feel about using time operations in examples since you can't just run them and repro the behavior without blocking the thread somehow (TestObserver in unit tests or doing the work on a different thread, etc).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer examples that can be tried quickly, thus examples requiring custom types are not good.

#### switchOnNext Example

```java
Observable<Observable<Long>> timeIntervals = Observable.interval(1, TimeUnit.MILLISECONDS)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not happy with this example - any suggestions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have two intertwined intervals. The outer 1 seconds, the inner 100 milliseconds and the inner could be mapped as outer + "-" + inner so that the switch over to a different value is more clear.


#### combineLatest Example

```java
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer examples that can be tried quickly, thus examples requiring custom types are not good.

#### switchOnNext Example

```java
Observable<Observable<Long>> timeIntervals = Observable.interval(1, TimeUnit.MILLISECONDS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have two intertwined intervals. The outer 1 seconds, the inner 100 milliseconds and the inner could be mapped as outer + "-" + inner so that the switch over to a different value is more clear.


## startWith

Emit a specified sequence of items before beginning to emit the items from the Observable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These descriptions are after Available and ReactiveX notes in #6131 btw.

@akarnokd akarnokd added this to the 3.0 milestone Aug 4, 2018
@akarnokd
Copy link
Member

akarnokd commented Aug 4, 2018

Thanks for your contribution and keep up the good work! 👍

I'll merge this once the Travis CI service works properly again.

@codecov
Copy link

codecov bot commented Aug 4, 2018

Codecov Report

Merging #6135 into 2.x will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6135      +/-   ##
============================================
- Coverage     98.31%   98.28%   -0.04%     
- Complexity     6192     6197       +5     
============================================
  Files           667      667              
  Lines         44853    44853              
  Branches       6213     6213              
============================================
- Hits          44097    44082      -15     
- Misses          228      232       +4     
- Partials        528      539      +11
Impacted Files Coverage Δ Complexity Δ
...ernal/operators/flowable/FlowableFlatMapMaybe.java 91.78% <0%> (-3.87%) 2% <0%> (ø)
...nternal/operators/observable/ObservableCreate.java 94.87% <0%> (-3.42%) 2% <0%> (ø)
...rnal/operators/flowable/FlowableFlatMapSingle.java 92.93% <0%> (-2.72%) 2% <0%> (ø)
...tivex/internal/schedulers/TrampolineScheduler.java 96.1% <0%> (-2.6%) 6% <0%> (ø)
...ernal/operators/flowable/FlowableFromIterable.java 94.11% <0%> (-2.14%) 5% <0%> (ø)
...java/io/reactivex/processors/PublishProcessor.java 98.19% <0%> (-1.81%) 42% <0%> (-1%)
.../operators/observable/ObservableFlatMapSingle.java 95.52% <0%> (-1.5%) 2% <0%> (ø)
...ternal/operators/observable/ObservableFlatMap.java 87.22% <0%> (-0.96%) 3% <0%> (ø)
...ernal/operators/flowable/FlowableTimeoutTimed.java 98.37% <0%> (-0.82%) 3% <0%> (ø)
...x/internal/operators/flowable/FlowableFlatMap.java 89.47% <0%> (-0.79%) 4% <0%> (ø)
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a58c491...86313ef. Read the comment docs.

@akarnokd akarnokd merged commit 3c27426 into ReactiveX:2.x Aug 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants