2.x: WIP removes anonymous inner classes. by SleimanJneidi · Pull Request #5174 · ReactiveX/RxJava

SleimanJneidi · 2017-03-11T16:09:20Z

This is a WIP. Since the changes are big, an incremental review and PR was recommended by @akarnokd

Removes them from flowable/observable/operators.
Issue #5150

codecov · 2017-03-11T16:24:07Z

Codecov Report

Merging #5174 into 2.x will decrease coverage by 0.03%.
The diff coverage is 98.78%.

@@             Coverage Diff             @@
##               2.x    #5174      +/-   ##
===========================================
- Coverage       96%   95.96%   -0.04%     
+ Complexity    5663     5654       -9     
===========================================
  Files          621      621              
  Lines        39974    40247     +273     
  Branches      5610     5610              
===========================================
+ Hits         38378    38625     +247     
- Misses         630      649      +19     
- Partials       966      973       +7

Impacted Files	Coverage Δ	Complexity Δ
...operators/flowable/FlowableWithLatestFromMany.java	`97.69% <100%> (+0.01%)`	`7 <0> (ø)`	❌
...al/operators/observable/ObservableSubscribeOn.java	`100% <100%> (ø)`	`2 <0> (ø)`	❌
...ternal/operators/flowable/FlowableBufferTimed.java	`90.94% <100%> (-0.07%)`	`5 <0> (ø)`
...vex/internal/operators/single/SingleDoOnEvent.java	`100% <100%> (ø)`	`2 <1> (ø)`	❌
...vex/internal/operators/flowable/FlowableDelay.java	`100% <100%> (ø)`	`3 <0> (ø)`	❌
...rnal/operators/flowable/FlowableCombineLatest.java	`92.46% <100%> (+0.03%)`	`8 <0> (ø)`	❌
...x/internal/operators/flowable/FlowablePublish.java	`96.03% <100%> (+1.86%)`	`10 <0> (+1)`	✅
...nternal/operators/observable/ObservableReplay.java	`97.74% <100%> (-0.14%)`	`18 <3> (-1)`
...x/internal/operators/single/SingleZipIterable.java	`100% <100%> (ø)`	`9 <0> (ø)`	❌
...internal/operators/observable/ObservableDelay.java	`100% <100%> (ø)`	`3 <0> (ø)`	❌
... and 82 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 f059ded...2377ea8. Read the comment docs.

akarnokd

Looks good so far, minus a couple of private modifiers that need to be removed.

akarnokd · 2017-03-11T16:30:18Z

src/main/java/io/reactivex/internal/operators/flowable/BlockingFlowableMostRecent.java

-                            throw ExceptionHelper.wrapOrThrow(NotificationLite.getError(buf));
-                        }
-                        return NotificationLite.getValue(buf);
+        private class Iterator implements java.util.Iterator<T> {


and no private

akarnokd · 2017-03-11T16:30:49Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableBufferTimed.java

            return true;
        }
+
+        private final class RemoveFromBuffer implements Runnable {


akarnokd · 2017-03-11T16:31:05Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableCombineLatest.java

        }
    }
+
+    private final class SingletonArrayFunc implements Function<T, R> {


akarnokd · 2017-03-11T16:31:15Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableDelay.java

            w.dispose();
        }

+        private final class OnNext implements Runnable {


akarnokd · 2017-03-11T16:31:25Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableDelay.java

+            }
+        }
+
+        private class OnError implements Runnable {


No private but final

akarnokd · 2017-03-11T16:34:33Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableTimeoutTimed.java

            dispose();
        }
+
+        private final class TimeoutTask implements Runnable {


akarnokd · 2017-03-11T16:34:51Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableUnsubscribeOn.java

+            }
+        }
+
+        private final class Cancellation implements Runnable {


akarnokd · 2017-03-11T16:35:04Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableWindowTimed.java

            }
        }
+
+        private final class Completion implements Runnable {


akarnokd · 2017-03-11T16:35:15Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableWithLatestFrom.java

        }
    }
+
+    private final class FlowableWithLatestSubscriber<U> implements FlowableSubscriber<U> {


akarnokd · 2017-03-11T16:35:29Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableWithLatestFromMany.java

        }
    }
+
+    private final class SingletonArrayFunc implements Function<T, R> {


akarnokd

A couple of places still require attention.

akarnokd · 2017-03-11T18:08:55Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableDelaySubscriptionOther.java

+        other.subscribe(otherSubscriber);
+    }
+
+    private final class DelaySubscriber implements FlowableSubscriber<U> {


Still private

akarnokd · 2017-03-11T18:09:07Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableDelaySubscriptionOther.java

+            main.subscribe(new OnCompleteSubscriber());
+        }
+
+        private class DelaySubscription implements Subscription {


no private but final

akarnokd · 2017-03-11T18:10:19Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableReplay.java

+            co.connect(new DisposableConsumer(srw));
+        }
+
+        private final class DisposableConsumer implements Consumer<Disposable> {


SleimanJneidi · 2017-03-11T18:17:44Z

@akarnokd on it

akarnokd

Still two missing. Look at the main page where there should be a bunch of collapsed comment along with the remaining problems in FlowableDelay.

akarnokd · 2017-03-11T18:26:31Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableDelay.java

+            }
+        }
+
+        private class OnError implements Runnable {


akarnokd · 2017-03-11T18:26:41Z

src/main/java/io/reactivex/internal/operators/flowable/FlowableDelay.java

+            }
+        }
+
+        private class OnComplete implements Runnable {


akarnokd · 2017-03-11T21:47:58Z

Great job! I'll merge this so the changes don't get too numerous.

removes anonymous inner classes from flowable

4db9ab5

akarnokd reviewed Mar 11, 2017

View reviewed changes

akarnokd added 2.x Cleanup labels Mar 11, 2017

address comments

5f6b53d

akarnokd reviewed Mar 11, 2017

View reviewed changes

akarnokd changed the title ~~WIP removes anonymous inner classes from flowable~~ 2.x: WIP removes anonymous inner classes from flowable Mar 11, 2017

Sleiman Jneidi added 2 commits March 11, 2017 18:23

remove remaining private classes

dca2f94

cleanups in observable

7769c37

SleimanJneidi changed the title ~~2.x: WIP removes anonymous inner classes from flowable~~ 2.x: WIP removes anonymous inner classes. Mar 11, 2017

akarnokd reviewed Mar 11, 2017

View reviewed changes

cleanups in operators

2377ea8

akarnokd merged commit 4c22f96 into ReactiveX:2.x Mar 11, 2017

akarnokd mentioned this pull request Mar 11, 2017

2.x: fix private field access, few generics problems #5176

Merged

SleimanJneidi deleted the refactor-remove-anonymous-inner-classes branch March 12, 2017 01:20

SleimanJneidi mentioned this pull request Mar 12, 2017

Removes all anonymous inner classes #5177

Merged

akarnokd mentioned this pull request Mar 24, 2017

2.0.8 release preparations #5220

Closed

michaelEllisUy mentioned this pull request Jun 1, 2017

Dependency update: rxjava xmartlabs/bigbang#82

Closed

Conversation

SleimanJneidi commented Mar 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

akarnokd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akarnokd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SleimanJneidi commented Mar 11, 2017

Uh oh!

akarnokd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akarnokd commented Mar 11, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SleimanJneidi commented Mar 11, 2017 •

edited

Loading

codecov bot commented Mar 11, 2017 •

edited

Loading