6

I am trying to implement "TaskExecutor" with generics (my first attempt to use generics) and using ExecutorService.

Here is my "TaskExecutor" class:

public class ExecuteAlerterTask<T> {
    public List<T> process(String executorName, Callable<T> task) throws ExecutionException, InterruptedException {
        final ThreadFactory threadFactory = new ThreadFactoryBuilder()
                .setNameFormat(executorName + "-%d")
                .setDaemon(true)
                .build();
        ExecutorService executor = Executors.newFixedThreadPool(10, threadFactory);
        Collection<Future<T>> futures = new ArrayList<>();
        IntStream.range(1, 10).forEach(i -> {
            Future<T> future = executor.submit(task);
            futures.add(future);
        });

        List<T> result = new ArrayList<>();
        for (Future<T> f : futures) {
            result.add(f.get());
        }
        executor.shutdown();
        return result;
    }
}

Here is my way to run it:

@Test
public void process() throws Exception {
    Callable<String> callable = () -> "Do something on ";
    ExecuteAlerterTask<String> executeAlerterTask = new ExecuteAlerterTask<>();
    List<String> result = executeAlerterTask.process("TaskName", callable);
    result.forEach(System.out::println);
}

And here is my question: How to write my Callable that it would accept argument i at a line:

Future<T> future = executor.submit(task);

E.g. desired result would be:

Do something on 1
Do something on 3
Do something on 7
Do something on 2
<...etc...>

If something else is wrong with my code - please let me know.

EDIT

Removed implements Callable

Code above is abstraction of what I really want to do:

  • IntRange really is set of batches where I get data from SQL. Callable really implements logic how to process those SQL batches.

EDIT2

After all suggestions I have following solution:

public class ExecuteAlerterTask<T> {
    public List<T> process(String executorName, Collection<Callable<T>> task) throws ExecutionException, InterruptedException {
        final ThreadFactory threadFactory = new ThreadFactoryBuilder()
                .setNameFormat(executorName + "-%d")
                .setDaemon(true)
                .build();
        ExecutorService executor = Executors.newFixedThreadPool(10, threadFactory);
        Collection<Future<T>> futures = executor.invokeAll(task);

        List<T> result = new ArrayList<>();
        for (Future<T> f : futures) {
            result.add(f.get());
        }
        executor.shutdown();
        return result;
    }
}

And way to run it:

   @Test
    public void process() throws Exception {
        Collection<Callable<String>> tasks = new ArrayList<>();
        IntStream.range(1, 10).forEach(i -> {
            tasks.add(new Task(i).callable);
        });

        ExecuteAlerterTask<String> executeAlerterTask = new ExecuteAlerterTask<>();
        List<String> result = executeAlerterTask.process("TaskName", tasks);
        result.forEach(System.out::println);
    }

    private class Task {
        private int i;
        private Callable<String> callable = () -> "Doing something on i: " + i;
        private Task(int i) {
            this.i = i;
        }
    }

EDIT3

Even simpler way to run it:

   @Test
    public void process() throws Exception {
        Collection<Callable<String>> tasks = new ArrayList<>();
        IntStream.range(1, 10).forEach(i -> {
            tasks.add(() -> "Do something on i: " + i * 2);
        });

        ExecuteAlerterTask<String> executeAlerterTask = new ExecuteAlerterTask<>();
        List<String> result = executeAlerterTask.process("TaskName", tasks);
        result.forEach(System.out::println);
    }

I guess I quite happy with final solution. Thanks all!

lapkritinis
  • 2,664
  • 4
  • 19
  • 29
  • 3
    Why are you implementing `Callable` when you don't use it? – RealSkeptic Sep 13 '17 at 11:48
  • @RealSkeptic Ok got it. That answers side question how to get rid of T call() – lapkritinis Sep 13 '17 at 11:50
  • It almost looks like you want to display on which thread work is performed, but you've written the code in an entirely backwards manner for that. – Kayaman Sep 13 '17 at 11:52
  • @Kayaman - maybe it looks like, but its not :) – lapkritinis Sep 13 '17 at 12:45
  • Maybe you should describe what you actually want to do, instead of writing code that looks like you're doing something in a completely wrong way. You're running 1 task, but you're expecting 10 different results. That's very dubious. – Kayaman Sep 13 '17 at 12:53
  • @Kayaman updated my question. In real world that one task operates on a batch of data. Int range would be: SELECT batch_id FROM unprocessedBatches. And Callable implements business logic what to do with that batch_id. – lapkritinis Sep 13 '17 at 12:58
  • 1
    With the new edit, it looks even more like you're doing it the wrong way. If you want to cleanly provide an Executor, the `Callable<>` you submit should have all the information it needs. The iteration over the batches needs to be done outside. Or you change the method to `process(Collection>)`. – daniu Sep 13 '17 at 13:00
  • @daniu thats why I asked question here - last statement "if something else is wrong with my code". I agree that Callable should have all information it needs. Should I submit that List tasks to **process**? – lapkritinis Sep 13 '17 at 13:03
  • @daniu thanks - I was thinking to take this approach. I thought maybe there is a "clean way" to add params to Callable, e.g. callable = (arg1, arg1) -> {"do something" + arg1}. But obviously not – lapkritinis Sep 13 '17 at 13:06
  • 1
    `Callable` is essentially a `Supplier`. You could use `Function` to provide a parameter and return a result, but it would be the executor who would have to provide the parameter. Then you'd have to provide a parameter supplier that would...yea, it could be done, but it wouldn't be pretty. – Kayaman Sep 13 '17 at 13:38
  • @daniu - does EDIT2 looks like right thing to do? – lapkritinis Sep 13 '17 at 13:53
  • 1
    @lapkritinis yes that looks simple enough - if you even need a class, just the lambda might be enough to produce the output. Depends on where the `i` comes from. – daniu Sep 13 '17 at 13:56

2 Answers2

3

Firstly, you don't need call() at all, neither do you need to implement Callable<T> in this class because you never use it as such.

To create a Callable the way you want, you could do:

Callable<String> task; // from the parameter
int i; // from the loop
Callable<String> wrapper = () -> { return task.call() + " on " + i; }
executor.submit(wrapper);

You're essentially giving the lambda the outside variable i as a parameter.

476rick
  • 2,764
  • 4
  • 29
  • 49
daniu
  • 14,137
  • 4
  • 32
  • 53
  • I see it would return what I have asked. Just thinking if it this solution would work on my "real thing" – lapkritinis Sep 13 '17 at 11:53
  • But this moves the "business logic" into TaskExecutor - this is something I would want to avoid – lapkritinis Sep 13 '17 at 12:01
  • 1
    Yes, but injecting the `i` parameter from the TaskExecutor into your business logic sounds like exactly what you're trying to do. I may have misunderstood your question; maybe you should post a "desired way to run" instead of an "desired result". – daniu Sep 13 '17 at 12:36
2

This is sort of impossible. You cannot pass a variable to a callable, if that's a lambda. On the other hand, you can use your own specific object that implements Callable and has a setter for the variable:

public class CallableWithParam implements Callable<String> {

    // protected for subclassing call()
    // volatile for multi-threaded reasons
    protected volatile int param = 0;

    public void setParam(int param) {
        this.param = param;
    }

    @Override
    public String call() {
        return "my param is: " + param;
    }

}

Usage:

@Test
public void process() throws Exception {
    CallableWithParam callable = new CallableWithParam() {
    @Override
        public String call() {
             // an anonymous inner class is almost a lambda ;)
            return "my param is: " + param + "in subclass";
        }
    };
    callable.setParam(3);
    ExecuteAlerterTask<String> executeAlerterTask = new ExecuteAlerterTask<>();
    List<String> result = executeAlerterTask.process("TaskName", callable);
    result.forEach(System.out::println);
}

Alternatively, you can set the param in the constructor instead of the setter.

Tamas Rev
  • 7,008
  • 5
  • 32
  • 49