(I'm not ignoring your points/questions - I will get back to them in a separate reply)
The name TaskGroup, in combination with the javadocs, made me think of a Group of Tasks that will be executed in parallel using the (default or optionally supplied) ExecutorService. I suppose I saw it as a ParallelTaskExecutor or TaskGroupExecutor, and not a ParallelTask or TaskGroupTask. This is probably where the initial confusion came from. Once you think of TaskGroup a just another Task, it follows that the ExecutorService provided in the constructor would be used to just run the execute() method of TaskGroup, as would happen with any other Task. So I might have approached it differently if the class names had an extra 'Task' appended to them (TaskGroupTask/TaskSequenceTask), and/or the javadocs said 'Task that runs...' rather than 'Class that runs ...' Chris On 27 July 2011 23:09, Greg Brown <gk_br...@verizon.net> wrote: >>> The Javadoc says pretty much the same thing as I wrote above: >> >> With the crucial exception of how they accomplish what they >> accomplish, and in particular, the use of a supplied ExecutorService. > > I agree that the Javadoc could provide more detail. But the fact that > TaskGroup and TaskSequence might use a different executor service honestly > never occurred to me, so I didn't mention it. > >>> TaskSequence - "Class that runs a sequence of tasks in series and notifies >>> listeners when all tasks are complete." >>> >>> TaskGroup - "Class that runs a group of tasks in parallel and notifies >>> listeners when all tasks are complete." >> >> I think you would agree that those single statements and the others in >> the javadocs are not sufficient to understand how the tasks are >> executed and therefore when it might be appropriate to use these >> classes ... > > Actually, I think they are sufficient. The only subtlety is which executor > service runs the tasks. Otherwise, the classes do exactly what the Javadoc > says. > > I think this may be more of an API design question than a documentation > issue. The question is - do you want to allow callers to execute sub-tasks in > a different executor service from the parent or not? Or, put another way, is > a developer likely to expect that sub-tasks are executed in the same executor > as the parent or not? > > If the former is true, then I think the existing API and documentation is > sufficient (though admittedly sparse). If the latter is true, then moving the > ExecutorService to the execute() method and adding more detail to the docs > would definitely be appropriate. > >
