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

Compose/Transform Covariance #1569

Merged

Conversation

benjchristensen
Copy link
Member

Failing test while exploring generic variance for #1416

Code like this makes these generics work:

    @Test
    public void testCovarianceOfCompose() {
        Observable<HorrorMovie> movie = Observable.<HorrorMovie> from(new HorrorMovie());
        movie.compose(new Transformer<Movie, Movie>() {

            @Override
            public Observable<? extends Movie> call(Observable<? super Movie> t1) {
                return Observable.from(new Movie());
            }

        });
    }

    @Test
    public void testCovarianceOfCompose2() {
        Observable<Movie> movie = Observable.<Movie> from(new HorrorMovie());
        movie.compose(new Transformer<Movie, Movie>() {

            @Override
            public Observable<? extends Movie> call(Observable<? super Movie> t1) {
                return Observable.from(new HorrorMovie());
            }

        });
    }

however, I couldn't get the compose/Transformer types to correctly work.

Anyone else want to help figure out generics for this?

See https://github.com/Netflix/RxJava/blob/master/rxjava-core/src/main/java/rx/Observable.java#L195

Failing test while exploring generic variance for ReactiveX#1416
@benjchristensen benjchristensen added this to the 0.20 milestone Aug 12, 2014
@cloudbees-pull-request-builder

RxJava-pull-requests #1481 FAILURE
Looks like there's a problem with this pull request

@benjchristensen
Copy link
Member Author

@abersnaze Thank you for fixing this ... I think I tried every permutation of super/extends except the right one that you came up with!

@cloudbees-pull-request-builder

RxJava-pull-requests #1486 SUCCESS
This pull request looks good

benjchristensen added a commit that referenced this pull request Aug 14, 2014
@benjchristensen benjchristensen merged commit 6c8ca2f into ReactiveX:master Aug 14, 2014
@benjchristensen benjchristensen deleted the compose-transformer-variance branch August 14, 2014 05:54
@zsxwing
Copy link
Member

zsxwing commented Aug 14, 2014

Returning Observable<? extends R> is not a good idea. The following codes won't be compiled:

    @Test
    public void testCovarianceOfCompose() {
        Observable<HorrorMovie> movie = Observable.<HorrorMovie> from(new HorrorMovie());
        Observable<Movie> movie2 =
                movie.compose(new Transformer<Movie, Movie>() {

            @Override
            public Observable<? extends Movie> call(Observable<? extends Movie> t1) {
                return Observable.from(new Movie());
            }

        });
    }

    @Test
    public void testCovarianceOfCompose2() {
        Observable<Movie> movie = Observable.<Movie> from(new HorrorMovie());
        Observable<HorrorMovie> movie2 =
                movie.compose(new Transformer<Movie, HorrorMovie>() {
            @Override
            public Observable<? extends HorrorMovie> call(Observable<? extends Movie> t1) {
                return Observable.from(new HorrorMovie());
            }
        });
    }

I propose zsxwing@855252b#diff-d41d8cd98f00b204e9800998ecf8427e just like other methods that avoids to return Observable<? extends R>.

@benjchristensen
Copy link
Member Author

Can you submit that as a PR?

Why in this use case must R not extend ? whereas the lift case does?

@zsxwing
Copy link
Member

zsxwing commented Aug 14, 2014

Please take a look at #1577

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants