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

Future reduce with executor, but this executor is never used #2375

Closed
DungBe opened this issue Feb 13, 2019 · 1 comment · Fixed by #2430
Closed

Future reduce with executor, but this executor is never used #2375

DungBe opened this issue Feb 13, 2019 · 1 comment · Fixed by #2430

Comments

@DungBe
Copy link

DungBe commented Feb 13, 2019

Future reduce with given executor! But this executor is never used.
For example: given executor is SimpleAsyncTaskExecutor, and the returned Future instance of this method contains DEFAULT_EXECUTOR

@danieldietrich danieldietrich added this to the v0.10.1 milestone Feb 13, 2019
@danieldietrich
Copy link
Contributor

Well spotted, thank you! We will need to patch Future#reduce like that:

static <T> Future<T> reduce(Executor executor, Iterable<? extends Future<? extends T>> futures, BiFunction<? super T, ? super T, ? extends T> f) {
    Objects.requireNonNull(executor, "executor is null");
    Objects.requireNonNull(futures, "futures is null");
    Objects.requireNonNull(f, "f is null");
    if (!futures.iterator().hasNext()) {
        throw new NoSuchElementException("Future.reduce on empty futures");
    } else {
-       return Future.<T> sequence(futures).map(seq -> seq.reduceLeft(f));
+       return Future.<T> sequence(executor, futures).map(seq -> seq.reduceLeft(f));
    }
}

thadumi pushed a commit to thadumi/vavr that referenced this issue Apr 10, 2019
The bug was about Future reduce with a given executor but instead of
this last one was used the default one.
@danieldietrich danieldietrich modified the milestones: v0.10.1, v0.11.0 Jul 2, 2019
@danieldietrich danieldietrich modified the milestones: v1.0.0, v0.10.1 Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants