patternjavaMinor
Java controller to farm out concurrent tasks
Viewed 0 times
farmjavacontrollertasksconcurrentout
Problem
I've recently given a coding interview on a Java concurrency task and unfortunately didn't get the job. The worst part is I've given my best but now I'm not even sure where went wrong. Can anyone help give me some ideas about things I can improve on below code?
The question is pretty vague. Given 4 generic interfaces which on a high level divides a task into small pieces, work on each piece and combine the partial result into final result. I'm asked to implement the central controller piece of the interface. The only requirement is to use concurrency in the partial result processing and "code must be production quality".
My code is as below (the interfaces were given). I did put in a lot of comment to explain my assumptions which are removed here.
```
// adding V,W in order to use in private fields types
public class ControllerImpl implements Controller {
private static Logger logger = LoggerFactory.getLogger(ControllerImpl.class);
private static int BATCH_SIZE = 100;
private Preprocessor preprocessor;
private Processor processor;
private Postprocessor postprocessor;
public ControllerImpl() {
this.preprocessor = new PreprocessorImpl<>();
this.processor = new ProcessorImpl<>();
this.postprocessor = new PostprocessorImpl<>();
}
public ControllerImpl(Preprocessor preprocessor, Processor processor, Postprocessor postprocessor) {
this.preprocessor = preprocessor;
this.processor = processor;
this.postprocessor = postprocessor;
}
@Override
public U process(T arg) {
if (arg == null) return null;
final V[] parts = preprocessor.split(arg);
final W[] partResult = (W[]) new Object[parts.length];
final int poolSize = Runtime.getRuntime().availableProcessors();
final ExecutorService executor = getExecutor(poolSize);
int i = 0;
while (i > tasks = IntStream.range(i, i + BATCH_SIZE)
.filter(e -> e (Cal
The question is pretty vague. Given 4 generic interfaces which on a high level divides a task into small pieces, work on each piece and combine the partial result into final result. I'm asked to implement the central controller piece of the interface. The only requirement is to use concurrency in the partial result processing and "code must be production quality".
My code is as below (the interfaces were given). I did put in a lot of comment to explain my assumptions which are removed here.
```
// adding V,W in order to use in private fields types
public class ControllerImpl implements Controller {
private static Logger logger = LoggerFactory.getLogger(ControllerImpl.class);
private static int BATCH_SIZE = 100;
private Preprocessor preprocessor;
private Processor processor;
private Postprocessor postprocessor;
public ControllerImpl() {
this.preprocessor = new PreprocessorImpl<>();
this.processor = new ProcessorImpl<>();
this.postprocessor = new PostprocessorImpl<>();
}
public ControllerImpl(Preprocessor preprocessor, Processor processor, Postprocessor postprocessor) {
this.preprocessor = preprocessor;
this.processor = processor;
this.postprocessor = postprocessor;
}
@Override
public U process(T arg) {
if (arg == null) return null;
final V[] parts = preprocessor.split(arg);
final W[] partResult = (W[]) new Object[parts.length];
final int poolSize = Runtime.getRuntime().availableProcessors();
final ExecutorService executor = getExecutor(poolSize);
int i = 0;
while (i > tasks = IntStream.range(i, i + BATCH_SIZE)
.filter(e -> e (Cal
Solution
Hints
I would have named the generic types properly.
Maybe I would have divided Logging from processing with help of a listener pattern.
I would have expected the input parameter to never be null, so returning null is obsolete.
I would have extracted some methods to have parts of code be named.
Code
On a very high level view I would have expected exactly what you described what the code should do. Splitting an input in parts, processing these parts in parallel, collect the results and aggregate them. This should have been expressed in the "process"-method.
I would have named the generic types properly.
Maybe I would have divided Logging from processing with help of a listener pattern.
I would have expected the input parameter to never be null, so returning null is obsolete.
I would have extracted some methods to have parts of code be named.
Code
On a very high level view I would have expected exactly what you described what the code should do. Splitting an input in parts, processing these parts in parallel, collect the results and aggregate them. This should have been expressed in the "process"-method.
public class Controller {
...
public RESULT process(INPUT input) {
PARTIALINPUT[] partialInputArray = preprocessor.split(input);
List> partialResultFutures = processAsynchronously(partialInputArray);
List partialResults = collectPartialResults(partialResultFutures);
RESULT result = postprocessor.aggregate(partialResults);
return result;
}
private List collectPartialResults(List> partialResultFutures)
throws InterruptedException, ExecutionException {
List partialResults = new ArrayList<>();
for (Future future : partialResultFutures) {
partialResults.add(future.get());
}
return partialResults;
}
private List> processAsynchronously(PARTIALINPUT[] partialInputArray) {
List> partialResultFutures = new ArrayList<>();
for (PARTIALINPUT partialinput : partialInputArray) {
Future future = processAsyncronously(partialInput);
partialResultFutures.add(future);
}
return partialResultFutures;
}
...
}Code Snippets
public class Controller<INPUT, RESULT, PARTIALINPUT, PARTIALRESULT> {
...
public RESULT process(INPUT input) {
PARTIALINPUT[] partialInputArray = preprocessor.split(input);
List<Future<PARTIALRESULT>> partialResultFutures = processAsynchronously(partialInputArray);
List<PARTIALRESULT> partialResults = collectPartialResults(partialResultFutures);
RESULT result = postprocessor.aggregate(partialResults);
return result;
}
private List<PARTIALRESULT> collectPartialResults(List<Future<PARTIALRESULT>> partialResultFutures)
throws InterruptedException, ExecutionException {
List<PARTIALRESULT> partialResults = new ArrayList<>();
for (Future<PARTIALRESULT> future : partialResultFutures) {
partialResults.add(future.get());
}
return partialResults;
}
private List<Future<PARTIALRESULT>> processAsynchronously(PARTIALINPUT[] partialInputArray) {
List<Future<PARTIALRESULT>> partialResultFutures = new ArrayList<>();
for (PARTIALINPUT partialinput : partialInputArray) {
Future<PARTIALRESULT> future = processAsyncronously(partialInput);
partialResultFutures.add(future);
}
return partialResultFutures;
}
...
}Context
StackExchange Code Review Q#134003, answer score: 3
Revisions (0)
No revisions yet.