Add Task.whenAllResult to collect results of multiple parallel tasks

[josephearl]

Creates a task that completes when all of the provided tasks are complete.

If none of the tasks faulted and none of the tasks were cancelled, the resulting task will end completed. The result of the returned task will be set to a list containing all of the results of the supplied tasks in the same order as they were provided (e.g. if the input tasks collection contained t1, t2, t3, the output task's result will return an List<TResult> where code list.get(0) == t1.getResult(), list.get(1) == t2.getResult(), and list.get(2) == t3.getResult()}.

[grantland]

Re: Task.whenAllTResult

This seems to make a drastic change to whenAll where we will now keep references to all results until all inner tasks are complete. This would be a problem where we want to launch a bunch of large file downloads where this might now hold all the files in memory until all the tasks complete, when we really just want to write to disk and lose the references to the files as they complete.

It would be better to call whenAll internally from whenAllTResult as opposed to the other way around and construct the result List in the continuation like:

public static Task<List<TResult>> whenAllTResult(final List<Task<TResult>> tasks) {
  return whenAll(tasks).onSuccess(
    // Get the results from tasks and return them in one list.
  );
}

This would prevent holding on to any unwanted references.

Re: Task.whenAllResult

I'm not sure we need this as I don't see many people needing to read the results of many tasks with different types and if they do, they can write one-off code like above. Additionally, it's not part of the .NET spec.

We can remove this and rename Task.whenAllTResult to Task.whenAllResult and simplify our APIs.

Re: AggregateException

I'm sorry about the List vs array change. This has been amended in #47 and it would be great to hear your thoughts on it.

[josephearl]

@grantland thanks for the feedback, will remove Task.whenAllResult and the changes to AggregateException and leave those for issue #47

I'll also make the recommended changes to whenAllTResult

[josephearl]

@grantland updated

[grantland]

Looks perfect! Once you squash your commits, I'll accept the PR.

[josephearl]

Squashed

[hallucinogen]

AggregateException will not only have java.lang.Exception, right? It might have java.lang.Error (basically it contains java.lang.Throwable)

[grantland]

For now I think it's fine as we only catch Exceptions right now. If/when we change it to Throwables, we'll change the documentation as well.

[josephearl]

Would we ever want to catch raw Error or Throwable? That would result in things like OOM error getting trapped rather than tearing down the process.

[grantland]

More or less to match with newer Java APIs as well as other libraries: ReactiveX/RxJava#296

[josephearl]

I'm both for and against. If it is implemented, a few errors need to be caught specially to make sure they are not swallowed. Also unobserved exception handling should probably be a pre-requisite for merging that change in order to ensure import errors are not forgotten about.

What happens if the users code encounters a StackOverflowError? We attempt to catch it, then most likely (if user code is running on the same thread) we encounter another StackOverflow attempting to run through the continuations. Except this time, the error comes from Task code so it looks like a library issue and the original error information is lost.

IMO the current situation already traps more things than it should, things like IllegalStateException and NullPointerException which we shouldn't - these errors indicate a programming mistake, not an execution failure and so they should not pollute the user error handling flow like they do currently.

However, too many developers have (over-)used RuntimeExceptions to indicate conditions that should not really crash a program because it saved time thinking about error handling so I don't think it's realistic to change that.

[josephearl]

ReactiveX/RxJava#748

[hallucinogen]

One comment, otherwise LGTM