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

Scan/Reduce Overloads & Type Erasure #1881

Closed
benjchristensen opened this issue Nov 15, 2014 · 5 comments
Closed

Scan/Reduce Overloads & Type Erasure #1881

benjchristensen opened this issue Nov 15, 2014 · 5 comments
Labels
Milestone

Comments

@benjchristensen
Copy link
Member

I think the recent additions to scan/reduce may cause issues

scan(R, Func2<R, ? super T, R>)
scan(Func0<R>, Func2<R, ? super T, R>)

The Func0 passed in looks like it can be treated like an Object and considered ambiguous and match with R instead of Func0.

Here is a compilation error:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project learnrxjava: Compilation failure: Compilation failure:
[ERROR] /Users/benjchristensen/development/github/learnrxjava/src/main/java/learnrxjava/examples/ScanVsReduceExample.java:[10,32] reference to reduce is ambiguous
[ERROR] both method <R>reduce(R,rx.functions.Func2<R,? super T,R>) in rx.Observable and method <R>reduce(rx.functions.Func0<R>,rx.functions.Func2<R,? super T,R>) in rx.Observable match
[ERROR] /Users/benjchristensen/development/github/learnrxjava/src/main/java/learnrxjava/examples/ScanVsReduceExample.java:[10,39] incompatible types: cannot infer type-variable(s) R
[ERROR] (argument mismatch; java.lang.Object is not a functional interface)
[ERROR] /Users/benjchristensen/development/github/learnrxjava/src/main/java/learnrxjava/examples/ScanVsReduceExample.java:[17,32] reference to scan is ambiguous
[ERROR] both method <R>scan(R,rx.functions.Func2<R,? super T,R>) in rx.Observable and method <R>scan(rx.functions.Func0<R>,rx.functions.Func2<R,? super T,R>) in rx.Observable match
[ERROR] /Users/benjchristensen/development/github/learnrxjava/src/main/java/learnrxjava/examples/ScanVsReduceExample.java:[17,37] incompatible types: cannot infer type-variable(s) R
[ERROR] (argument mismatch; java.lang.Object is not a functional interface)
[ERROR] -> [Help 1]

Here is example code: https://github.com/jhusain/learnrxjava/blob/master/src/main/java/learnrxjava/examples/ScanVsReduceExample.java

package learnrxjava.examples;

import java.util.ArrayList;

import rx.Observable;

public class ScanVsReduceExample {

    public static void main(String... args) {
        Observable.range(0, 10).reduce(() -> new ArrayList<Integer>(), (list, i) -> {
            list.add(i);
            return list;
        }).forEach(System.out::println);

        System.out.println("... vs ...");

        Observable.range(0, 10).scan(() -> new ArrayList<Integer>(), (list, i) -> {
            list.add(i);
            return list;
        }).forEach(System.out::println);
    }
}

It looks like we need to do one of 3 things:

  1. Remove one of several things:
scan(R, Func2<R, ? super T, R>)
scan(Func0<R>, Func2<R, ? super T, R>)
  1. Rename one of them

  2. Add an extra argument so arity solves it.

I actually think the most correct thing to do is remove scan(R, Func2<R, ? super T, R>) since an initial value is most often intended for mutable state in scan/reduce.

cc @headinthebox as this is a last minute API fix we need prior to Monday for 1.0

@benjchristensen benjchristensen added this to the 1.0 milestone Nov 15, 2014
@headinthebox
Copy link
Contributor

I'd be happy with Func<0> as the only way, but then we have to do the same for reduce and anything else that takes a seed.

@headinthebox
Copy link
Contributor

BTW, if we do this, then we should consider generalizing to returning S since we are side-effecting R, so it does not matter to return return the mutable state variable since that is passed in by the implementation.

scan(Func0<R>, Func2<R, ? super T, S>)

benjchristensen added a commit to benjchristensen/RxJava that referenced this issue Nov 15, 2014
This fixes the Java 8 ambiguity between the overloads: ReactiveX#1881
@benjchristensen
Copy link
Member Author

I have submitted #1883 to fix the ambiguity that happens with lambdas.

I have changed scan, reduce and collect. I don't see anywhere else that needs to change.

@benjchristensen
Copy link
Member Author

I'm going to pull the factory overload until 1.1 so we make sure we get this right.

benjchristensen added a commit to benjchristensen/RxJava that referenced this issue Nov 15, 2014
This puts the seed factory on `collect` and removes it from `scan` and `reduce` due to ambiguity.
See ReactiveX#1883 and ReactiveX#1881
@benjchristensen
Copy link
Member Author

Fixed in #1884 by deleting the ambiguous new factory overloads for scan and reduce and modifying collect to support the mutable case with a seed factory and side-affection action.

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

No branches or pull requests

2 participants