There is no need for CompletableFuture, as the way you use the ExecutorService is sufficient, though, there are some aspects of the code flow which could be improved. You fetch the first element, even when not needed, and you cast it to CallableService for no reason, as you can call the method via the Callable interface already. In the other branch you are catching InterruptedException and proceeding, so the caller would never know that not all jobs have been executed. And in a straight-forward code flow, you don't need to check the list for null:
public static void execute(List<Callable<Boolean>> threadList) throws Exception {
if(1 == threadList.size()) {
Callable<Boolean> singleService = threadList.get(0);
singleService.call();
}
else {
List<Future<Boolean>> futureList = myThreadPoolExecutor.invokeAll(threadList);
for(Future<Boolean> future : futureList) {
try {
future.get();
}
catch(Exception e) {
//do some calculations here and then throw exception
throw new Exception(e.getMessage(), e);
}
}
}
}
You could shorten it further to
public static void execute(List<Callable<Boolean>> threadList) throws Exception {
if(1 == threadList.size()) {
threadList.get(0).call();
}
else {
for(Future<Boolean> future : myThreadPoolExecutor.invokeAll(threadList)) {
try {
future.get();
}
catch(Exception e) {
//do some calculations here and then throw exception
throw new Exception(e.getMessage(), e);
}
}
}
}
But that's a matter of preferred coding style. But note that it caught my eye that in the single element case, you're not performing the same exception handling.
To use CompletableFuture, we need an adapter method, as the convenience method supplyAsync requires a Supplier instead of a Callable. Using a modified variant of this answer, we get
public static void execute(List<Callable<Boolean>> threadList) throws Exception {
if(1 == threadList.size()) {
threadList.get(0).call();
}
else {
CompletableFuture<?> all = CompletableFuture.allOf(
threadList.stream()
.map(c -> callAsync(c, myThreadPoolExecutor))
.toArray(CompletableFuture<?>[]::new));
try {
all.get();
}
catch(Exception e) {
//do some calculations here and then throw exception
throw new Exception(e.getMessage(), e);
}
}
}
public static <R> CompletableFuture<R> callAsync(Callable<R> callable, Executor e) {
CompletableFuture<R> cf = new CompletableFuture<>();
CompletableFuture.runAsync(() -> {
try { cf.complete(callable.call()); }
catch(Throwable ex) { cf.completeExceptionally(ex); }
}, e);
return cf;
}
So we have no invokeAll which takes care of submitting all jobs. We have to do this manually, either with a loop or a stream operation. On the other hand, we get a single future via allOf representing the completion status, exceptionally if at least one job failed.
Unlike invokeAll, which waits for the completion, allOf only returns the future so it is the all.get() call which waits for the completion. We could do other things before it or even use this property to always perform the first job in the caller thread:
public static void execute(List<Callable<Boolean>> threadList) throws Exception {
CompletableFuture<?> tail = CompletableFuture.allOf(
threadList.stream().skip(1)
.map(c -> callAsync(c, myThreadPoolExecutor))
.toArray(CompletableFuture<?>[]::new)),
head = callAsync(threadList.get(0), Runnable::run);
try {
head.get();
tail.get();
}
catch(Exception e) {
//do some calculations here and then throw exception
throw new Exception(e.getMessage(), e);
}
}
This will always call the first callable in the current thread, as Runnable::run used as Executor will perform the action immediately in the calling thread. But it's treated uniformly in all other aspects, especially the exception handling. When there is only one job, allOf invoke with an empty array will do nothing and return an already completed future, which will have the desired effect.
