Make it explicit that throttleWithTimout is an alias of debounce #6049

romanzes · 2018-06-17T11:47:31Z

The documentation implies that these two methods are somewhat different. Fixing that confusion, as discussed in #6043:

Made the descriptions of these methods the same
Mentioned that it is an alias in the first paragraphs of throttleWithTimeout javadoc comments
Removed the links to third-party websites

akarnokd · 2018-06-17T12:25:27Z

src/main/java/io/reactivex/Flowable.java

     * @param scheduler
     *            the {@link Scheduler} to use internally to manage the timers that handle the timeout for each
     *            item
     * @return a Flowable that filters out items from the source Publisher that are too quickly followed by
     *         newer items
     * @see <a href="http://reactivex.io/documentation/operators/debounce.html">ReactiveX operators documentation: Debounce</a>
     * @see <a href="https://github.com/ReactiveX/RxJava/wiki/Backpressure">RxJava wiki: Backpressure</a>
-     * @see #throttleWithTimeout(long, TimeUnit, Scheduler)


Please keep these for clarity.

akarnokd · 2018-06-17T12:27:33Z

src/main/java/io/reactivex/Flowable.java

@@ -15774,34 +15761,31 @@ public final Completable switchMapCompletableDelayError(@NonNull Function<? supe
    }

    /**
-     * Returns a Flowable that only emits those items emitted by the source Publisher that are not followed
-     * by another emitted item within a specified time window.
+     * Alias to {@link #debounce(long, TimeUnit)}


Please keep the description as the first sentence and these as the second one or as a braced comment on the first sentence.

Returns a Flowable that only emits those items emitted by the source Publisher that are not followed by another emitted item within a specified time window (alias to {@link #debounce(long, TimeUnit)}).

akarnokd · 2018-06-17T12:28:20Z

src/main/java/io/reactivex/Flowable.java

     * <dl>
     *  <dt><b>Backpressure:</b></dt>
     *  <dd>This operator does not support backpressure as it uses time to control data flow.</dd>
     *  <dt><b>Scheduler:</b></dt>
-     *  <dd>{@code throttleWithTimeout} operates by default on the {@code computation} {@link Scheduler}.</dd>
+     *  <dd>This version of {@code throttleWithTimeout} operates by default on the {@code computation} {@link Scheduler}.</dd>


There is no need to change this.

Agree, this is too verbose. Reverted it here and also changed javadoc for debounce which also had this unnecessary verbosity.

akarnokd · 2018-06-17T12:28:57Z

src/main/java/io/reactivex/Flowable.java

-     * Returns a Flowable that only emits those items emitted by the source Publisher that are not followed
-     * by another emitted item within a specified time window, where the time window is governed by a specified
-     * Scheduler.
+     * Alias to {@link #debounce(long, TimeUnit, Scheduler)}


Please keep the original first sentence, similarly as above.

moved the mention of alias to the end of the sentence

akarnokd · 2018-06-17T12:29:49Z

src/main/java/io/reactivex/Observable.java

@@ -13180,32 +13167,30 @@ public final Completable switchMapCompletableDelayError(@NonNull Function<? supe
    }

    /**
-     * Returns an Observable that only emits those items emitted by the source ObservableSource that are not followed
-     * by another emitted item within a specified time window.
+     * Alias to {@link #debounce(long, TimeUnit, Scheduler)}


Same situation, please keep the first sentence and mention it being an alias later.

akarnokd · 2018-06-17T12:30:17Z

src/main/java/io/reactivex/Observable.java

-     * Returns an Observable that only emits those items emitted by the source ObservableSource that are not followed
-     * by another emitted item within a specified time window, where the time window is governed by a specified
-     * Scheduler.
+     * Alias to {@link #debounce(long, TimeUnit, Scheduler)}


Please keep the original sentence first.

akarnokd · 2018-06-17T12:30:49Z

src/main/java/io/reactivex/Observable.java

     * @param scheduler
     *            the {@link Scheduler} to use internally to manage the timers that handle the timeout for each
     *            item
     * @return an Observable that filters out items from the source ObservableSource that are too quickly followed by
     *         newer items
     * @see <a href="http://reactivex.io/documentation/operators/debounce.html">ReactiveX operators documentation: Debounce</a>
-     * @see #throttleWithTimeout(long, TimeUnit, Scheduler)


Please keep the see reference.

akarnokd · 2018-06-17T12:38:22Z

src/main/java/io/reactivex/Flowable.java

     * <p>
-     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">


Please keep the marbles as they were because they contain the operator name.

Okay, done. However, I don't understand why there should be two copies of the same image with different names. Take a look at the javadoc for share() operator: it is an alias for publish().refCount() and it has a link to https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/publishRefCount.png.

There are a lot of copy-paste and outdated diagrams to fix because of wrong operator names. At least these were named according to their operator. Leaving it as is the simpler task as the alternative, combining the two image by listning both names would be a different undertaking.

akarnokd · 2018-06-17T12:38:54Z

src/main/java/io/reactivex/Flowable.java

     * <p>
-     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.s.png" alt="">


Please keep the original marble here.

akarnokd · 2018-06-17T12:43:37Z

src/main/java/io/reactivex/Observable.java

     * <dl>
     *  <dt><b>Scheduler:</b></dt>
-     *  <dd>{@code throttleWithTimeout} operates by default on the {@code computation} {@link Scheduler}.</dd>
+     *  <dd>This version of {@code debounce} operates by default on the {@code computation} {@link Scheduler}.</dd>


There is no reason to change this line, also the method name is incorrect: https://travis-ci.org/ReactiveX/RxJava/builds/393285007#L649

my bad, fixed

akarnokd · 2018-06-17T12:44:14Z

Build warnings: please avoid dangling <p> tags: https://travis-ci.org/ReactiveX/RxJava/builds/393285007#L476

romanzes

@akarnokd thanks for your feedback. Please take a look again.

romanzes · 2018-06-17T15:59:04Z

src/main/java/io/reactivex/Flowable.java

     * @param scheduler
     *            the {@link Scheduler} to use internally to manage the timers that handle the timeout for each
     *            item
     * @return a Flowable that filters out items from the source Publisher that are too quickly followed by
     *         newer items
     * @see <a href="http://reactivex.io/documentation/operators/debounce.html">ReactiveX operators documentation: Debounce</a>
     * @see <a href="https://github.com/ReactiveX/RxJava/wiki/Backpressure">RxJava wiki: Backpressure</a>
-     * @see #throttleWithTimeout(long, TimeUnit, Scheduler)


romanzes · 2018-06-17T15:59:12Z

src/main/java/io/reactivex/Flowable.java

@@ -15774,34 +15761,31 @@ public final Completable switchMapCompletableDelayError(@NonNull Function<? supe
    }

    /**
-     * Returns a Flowable that only emits those items emitted by the source Publisher that are not followed
-     * by another emitted item within a specified time window.
+     * Alias to {@link #debounce(long, TimeUnit)}


romanzes · 2018-06-17T16:15:32Z

src/main/java/io/reactivex/Flowable.java

     * <p>
-     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">


Okay, done. However, I don't understand why there should be two copies of the same image with different names. Take a look at the javadoc for share() operator: it is an alias for publish().refCount() and it has a link to https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/publishRefCount.png.

romanzes · 2018-06-17T16:21:36Z

src/main/java/io/reactivex/Flowable.java

     * <dl>
     *  <dt><b>Backpressure:</b></dt>
     *  <dd>This operator does not support backpressure as it uses time to control data flow.</dd>
     *  <dt><b>Scheduler:</b></dt>
-     *  <dd>{@code throttleWithTimeout} operates by default on the {@code computation} {@link Scheduler}.</dd>
+     *  <dd>This version of {@code throttleWithTimeout} operates by default on the {@code computation} {@link Scheduler}.</dd>


Agree, this is too verbose. Reverted it here and also changed javadoc for debounce which also had this unnecessary verbosity.

romanzes · 2018-06-17T16:22:24Z

src/main/java/io/reactivex/Flowable.java

-     * Returns a Flowable that only emits those items emitted by the source Publisher that are not followed
-     * by another emitted item within a specified time window, where the time window is governed by a specified
-     * Scheduler.
+     * Alias to {@link #debounce(long, TimeUnit, Scheduler)}


moved the mention of alias to the end of the sentence

romanzes · 2018-06-17T16:22:29Z

src/main/java/io/reactivex/Flowable.java

     * <p>
-     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.s.png" alt="">


romanzes · 2018-06-17T16:22:38Z

src/main/java/io/reactivex/Observable.java

     * @param scheduler
     *            the {@link Scheduler} to use internally to manage the timers that handle the timeout for each
     *            item
     * @return an Observable that filters out items from the source ObservableSource that are too quickly followed by
     *         newer items
     * @see <a href="http://reactivex.io/documentation/operators/debounce.html">ReactiveX operators documentation: Debounce</a>
-     * @see #throttleWithTimeout(long, TimeUnit, Scheduler)


romanzes · 2018-06-17T16:22:48Z

src/main/java/io/reactivex/Observable.java

@@ -13180,32 +13167,30 @@ public final Completable switchMapCompletableDelayError(@NonNull Function<? supe
    }

    /**
-     * Returns an Observable that only emits those items emitted by the source ObservableSource that are not followed
-     * by another emitted item within a specified time window.
+     * Alias to {@link #debounce(long, TimeUnit, Scheduler)}


romanzes · 2018-06-17T16:23:10Z

src/main/java/io/reactivex/Observable.java

     * <dl>
     *  <dt><b>Scheduler:</b></dt>
-     *  <dd>{@code throttleWithTimeout} operates by default on the {@code computation} {@link Scheduler}.</dd>
+     *  <dd>This version of {@code debounce} operates by default on the {@code computation} {@link Scheduler}.</dd>


my bad, fixed

romanzes · 2018-06-17T16:23:40Z

src/main/java/io/reactivex/Observable.java

-     * Returns an Observable that only emits those items emitted by the source ObservableSource that are not followed
-     * by another emitted item within a specified time window, where the time window is governed by a specified
-     * Scheduler.
+     * Alias to {@link #debounce(long, TimeUnit, Scheduler)}


codecov · 2018-06-17T16:47:58Z

Codecov Report

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

@@             Coverage Diff              @@
##                2.x    #6049      +/-   ##
============================================
- Coverage     98.27%   98.25%   -0.03%     
- Complexity     6190     6192       +2     
============================================
  Files           666      666              
  Lines         44802    44802              
  Branches       6206     6206              
============================================
- Hits          44030    44019      -11     
- Misses          236      239       +3     
- Partials        536      544       +8

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/io/reactivex/Flowable.java	`100% <ø> (ø)`	`564 <0> (ø)`	⬇️
src/main/java/io/reactivex/Observable.java	`100% <ø> (ø)`	`539 <0> (ø)`	⬇️
...reactivex/internal/operators/single/SingleAmb.java	`96.36% <0%> (-3.64%)`	`9% <0%> (-1%)`
...ernal/operators/flowable/FlowableFlatMapMaybe.java	`92.27% <0%> (-3.39%)`	`2% <0%> (ø)`
.../operators/observable/ObservableFlatMapSingle.java	`88.8% <0%> (-2.99%)`	`2% <0%> (ø)`
...ternal/operators/completable/CompletableMerge.java	`96.42% <0%> (-2.39%)`	`2% <0%> (ø)`
...rnal/operators/flowable/FlowableFlatMapSingle.java	`94.56% <0%> (-2.18%)`	`2% <0%> (ø)`
.../io/reactivex/internal/schedulers/IoScheduler.java	`89.24% <0%> (-2.16%)`	`9% <0%> (ø)`
.../io/reactivex/disposables/CompositeDisposable.java	`98.14% <0%> (-1.86%)`	`39% <0%> (-1%)`
...perators/mixed/ObservableConcatMapCompletable.java	`98.49% <0%> (-1.51%)`	`3% <0%> (ø)`
... and 21 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 65c4956...4dc8b76. Read the comment docs.

akarnokd

Almost done, some marble image references are wrong.

akarnokd · 2018-06-17T16:52:01Z

src/main/java/io/reactivex/Observable.java

     * <p>
-     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">
+     * <img width="640" height="310" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/debounce.png" alt="">


Please use throttleWithTimeout image.

akarnokd · 2018-06-17T16:52:34Z

src/main/java/io/reactivex/Observable.java

-     * <li><a href="http://unscriptable.com/2009/03/20/debouncing-javascript-methods/">Debouncing: javascript methods</a></li>
-     * <li><a href="http://www.illyriad.co.uk/blog/index.php/2011/09/javascript-dont-spam-your-server-debounce-and-throttle/">Javascript - don't spam your server: debounce and throttle</a></li>
-     * </ul>
+     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">


This should be throttleWithTimeout.s.png.

akarnokd · 2018-06-17T16:54:36Z

src/main/java/io/reactivex/Flowable.java

-     * <li><a href="http://unscriptable.com/2009/03/20/debouncing-javascript-methods/">Debouncing: javascript methods</a></li>
-     * <li><a href="http://www.illyriad.co.uk/blog/index.php/2011/09/javascript-dont-spam-your-server-debounce-and-throttle/">Javascript - don't spam your server: debounce and throttle</a></li>
-     * </ul>
+     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">


This should be throttleWithTimeout.s.png.

Oh, didn't notice there are two versions. Fixed.

akarnokd · 2018-06-17T16:57:00Z

src/main/java/io/reactivex/Observable.java

@@ -13259,7 +13236,7 @@ public final Completable switchMapCompletableDelayError(@NonNull Function<? supe
     * Returns an Observable that emits records of the time interval between consecutive items emitted by the
     * source ObservableSource.
     * <p>
-     * <img width="640" height="310" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/timeInterval.png" alt="">
+     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">


This looks like a mistake as this is the docs for timeInterval.

Yeah, shouldn't have worked on it at 2am... Done.

romanzes

I apologize for my inattentiveness. Should be fine now :)

romanzes · 2018-06-18T00:08:58Z

src/main/java/io/reactivex/Flowable.java

-     * <li><a href="http://unscriptable.com/2009/03/20/debouncing-javascript-methods/">Debouncing: javascript methods</a></li>
-     * <li><a href="http://www.illyriad.co.uk/blog/index.php/2011/09/javascript-dont-spam-your-server-debounce-and-throttle/">Javascript - don't spam your server: debounce and throttle</a></li>
-     * </ul>
+     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">


Oh, didn't notice there are two versions. Fixed.

romanzes · 2018-06-18T00:09:48Z

src/main/java/io/reactivex/Observable.java

     * <p>
-     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">
+     * <img width="640" height="310" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/debounce.png" alt="">


romanzes · 2018-06-18T00:13:58Z

src/main/java/io/reactivex/Observable.java

-     * <li><a href="http://unscriptable.com/2009/03/20/debouncing-javascript-methods/">Debouncing: javascript methods</a></li>
-     * <li><a href="http://www.illyriad.co.uk/blog/index.php/2011/09/javascript-dont-spam-your-server-debounce-and-throttle/">Javascript - don't spam your server: debounce and throttle</a></li>
-     * </ul>
+     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">


romanzes · 2018-06-18T00:14:44Z

src/main/java/io/reactivex/Observable.java

@@ -13259,7 +13236,7 @@ public final Completable switchMapCompletableDelayError(@NonNull Function<? supe
     * Returns an Observable that emits records of the time interval between consecutive items emitted by the
     * source ObservableSource.
     * <p>
-     * <img width="640" height="310" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/timeInterval.png" alt="">
+     * <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">


Yeah, shouldn't have worked on it at 2am... Done.

Make it explicit that throttleWithTimout is an alias of debounce

a59b6cd

akarnokd requested changes Jun 17, 2018

View reviewed changes

akarnokd added Documentation 2.x labels Jun 17, 2018

akarnokd added this to the 2.2 milestone Jun 17, 2018

akarnokd reviewed Jun 17, 2018

View reviewed changes

feedback

6fc4d0a

romanzes commented Jun 17, 2018

View reviewed changes

akarnokd requested changes Jun 17, 2018

View reviewed changes

fixed marble images

4dc8b76

romanzes commented Jun 18, 2018

View reviewed changes

akarnokd approved these changes Jun 18, 2018

View reviewed changes

vanniktech approved these changes Jun 18, 2018

View reviewed changes

akarnokd merged commit e6b852d into ReactiveX:2.x Jun 18, 2018

This was referenced Jun 18, 2018

2.1.15 release preparations #6048

Closed

2.x: Observable.throttleWithTimeout is confusing #6043

Closed

jared-bot mentioned this pull request Jul 2, 2018

Dependency update: RxJava xmartlabs/bigbang-template#105

Closed

jared-bot mentioned this pull request Jul 23, 2018

Dependency update: RxJava xmartlabs/bigbang-template#113

Closed

jared-bot mentioned this pull request Jul 31, 2018

Dependency update: RxJava xmartlabs/bigbang-template#117

Merged

		* <p>
		* <img width="640" height="305" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleWithTimeout.png" alt="">

Make it explicit that throttleWithTimout is an alias of debounce #6049

Make it explicit that throttleWithTimout is an alias of debounce #6049

Conversation

romanzes commented Jun 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akarnokd commented Jun 17, 2018

romanzes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 17, 2018 • edited Loading

Codecov Report

akarnokd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romanzes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romanzes commented Jun 17, 2018 •

edited

Loading

codecov bot commented Jun 17, 2018 •

edited

Loading