Skip to content

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

Closed
@ggikko

Description

@ggikko
Contributor

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?

Activity

JakeWharton

JakeWharton commented on Mar 27, 2017

@JakeWharton
Contributor

That would be a breaking change.

ggikko

ggikko commented on Mar 27, 2017

@ggikko
ContributorAuthor

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

akarnokd

akarnokd commented on Mar 27, 2017

@akarnokd
Member

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

ggikko commented on Mar 27, 2017

@ggikko
ContributorAuthor

good comment! Thanks :)

TommyLemon

TommyLemon commented on Apr 7, 2017

@TommyLemon

@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

akarnokd commented on Apr 7, 2017

@akarnokd
Member

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

TommyLemon commented on Apr 12, 2017

@TommyLemon

@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

TommyLemon commented on Apr 12, 2017

@TommyLemon

@akarnokd thank you~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @JakeWharton@akarnokd@TommyLemon@ggikko

        Issue actions

          I'm qurious about some codes. can anyone comments for me? · Issue #5239 · ReactiveX/RxJava