ReactiveX / RxJava

RxJava – Reactive Extensions for the JVM – a library for composing asynchronous and event-based programs using observable sequences for the Java VM.
Apache License 2.0
47.94k stars 7.6k forks source link

I'm qurious about some codes. can anyone comments for me? #5239

Closed ggikko closed 7 years ago

ggikko commented 7 years ago

There are many such method in Observable.java like..

    public static <T> Observable<T> just(T item1, T item2) {
        ObjectHelper.requireNonNull(item1, "The first item is null");
        ObjectHelper.requireNonNull(item2, "The second item is null");

        return fromArray(item1, item2);
    }
   public static <T> Observable<T> just(T item1, T item2, T item3) {
        ObjectHelper.requireNonNull(item1, "The first item is null");
        ObjectHelper.requireNonNull(item2, "The second item is null");
        ObjectHelper.requireNonNull(item3, "The third item is null");

        return fromArray(item1, item2, item3);
    }

1...10

I think this method is enough to cover such a case.


public static <T> Observable<T> just(T... items) {
        for (T item : items) {
            ObjectHelper.requireNonNull(item, "The item is null");
        }
        return fromArray(items);
    }

or add index for explicit error message.

can I change previous methods to like one?

JakeWharton commented 7 years ago

That would be a breaking change.

ggikko commented 7 years ago

yeah.. I agree :) thanks for your comment.

akarnokd commented 7 years ago

First, those methods exist so you don't have to suppress the unchecked exception from using varargs on a generic type for typical 1-9 argument cases. Second, if items is long, you have two loops over it instead of one. Third, the proposed method doesn't tell which element was null. Fourth, that just(...) would cause ambiguity problems with the other existing ones; this is why fromArray has been introduced.

ggikko commented 7 years ago

good comment! Thanks :)

TommyLemon commented 7 years ago

@ggikko @akarnokd Hi I'm an Android developer and are learning RxJava now. I have the same question and I think neither of the last 2 reasons is reasonable. 3.We can change the proposed code like this:

    public static <T> Observable<T> just(T... items) {
        int count = items == null ? 0 : items.length;
        for (int i = 0; i < count; i++) {
            ObjectHelper.requireNonNull(items[i], "The items[" + i + "] is null");
        }
        return fromArray(items);
    }

4.The just methods given in Observable.java have the same problem too.

akarnokd commented 7 years ago

3: now you have string concatenation overhead as well. When looking at an API, one usually doesn't see the evolution of it. Many RxJava 2 API design choices were established from the experience with the initial Java 8 target where ambiguities were very common. Plus, the new version allowed a design of an API where all operators and overloads have been known upfront and the naming and distribution of them could be done more consistently (instead of the tagged-on feeling of many 1.x operators).

TommyLemon commented 7 years ago

@ggikko @akarnokd So this is a historical issue? Will you fix it this year? Anyway I still fell love with it after several days learning recently and made a RxJava example here: all_moments moment_detail using_rxjava

https://github.com/TommyLemon/APIJSON-Android-RxJava

TommyLemon commented 7 years ago

@akarnokd thank you~