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 with Seed Factory #1835

Merged

Conversation

benjchristensen
Copy link
Member

Adds overload with seed factory as per #1831

Adds overload with seed factory as per ReactiveX#1831
@benjchristensen
Copy link
Member Author

cc @headinthebox for review

@benjchristensen
Copy link
Member Author

Merging for another release candidate to get actual testing during this next week.

benjchristensen added a commit that referenced this pull request Nov 8, 2014
@benjchristensen benjchristensen merged commit c6c45f0 into ReactiveX:1.x Nov 8, 2014
@headinthebox
Copy link
Contributor

For the mutable case, I think it is better to pass a special object such that we can hide the state, and all methods can access it without passing it as a parameter. Think something along the lines of http://msdn.microsoft.com/en-us/library/ms131051.aspx

@benjchristensen benjchristensen deleted the reduce-scan-mutable-seed branch November 11, 2014 03:33
@benjchristensen
Copy link
Member Author

A special object means we can't use lambdas. The point of scan/reduce for most use cases I see is for accumulating state in a mutable object, so not supporting use of lambdas for the mutable case effectively says scan would require using an anonymous inner class. That seems unnecessary when I can write code like this:

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

@headinthebox
Copy link
Contributor

Yup, it would require an anonymous inner class, but it is also what Java 8 does https://docs.oracle.com/javase/8/docs/api/java/util/stream/Collector.html, https://docs.oracle.com/javase/8/docs/api/java/util/stream/Collectors.html.

@benjchristensen
Copy link
Member Author

I'm okay if that's an extra overload, but I wouldn't want it to be the only way. It's very awkward using an anonymous inner class when nothing else requires that.

Also, we'd have to figure out for the first time where to put a new type like this that is custom to a specific operator.

@headinthebox
Copy link
Contributor

100% agree, but I think it makes sense to adopt the Java8 overload.

@benjchristensen
Copy link
Member Author

We can't as we compile to Java 6.

@benjchristensen
Copy link
Member Author

We had to revert this because of ambiguity. See #1884 for the change.

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.

2 participants