public class TaskA implements Runnable {
....
}
public class TaskB implements Runnable {
....
}
I have two runnable class as TaskA, TaskB. I want to have a TaskManager to run these two tasks and have possibility to add more task in the future. So I have the TaskManager as following:
public class TaskManager {
private ThreadPoolExecutor threadPool;
private Object task;
public TaskManager(TaskA task) {
this.task = task;
}
public TaskManager(TaskB task) {
this.task = task;
}
public void startThreadPool() {
threadPool = new ThreadPoolExecutor(...);
threadPool.execute((Runnable) task);
}
}
I would like to know whether it is a good practice? If not, anyone have any suggestion? should we use consumer or so to implement this one?
2 Answers 2
Since it seems you only use task as a Runnable
internally, use Runnable instead of Object. Change your third line to:
private final Runnable task;
Simple, saves you a cast later. And I'd consider renaming it from "task" to "runnable" for clarity, if that makes more sense.
-
Changing it to a
Runnable
, yes. Renaming it?... Very debatable.svidgen– svidgen2017年03月07日 16:31:29 +00:00Commented Mar 7, 2017 at 16:31 -
Valid point, edited the answer.user949300– user9493002017年03月07日 16:42:25 +00:00Commented Mar 7, 2017 at 16:42
TLDR:
Use just one constructor that accepts a Runnable
and a member of type Runnable
.
Long answer:
If you really, really need to have two difrerent constructors, one that receives a TaskA
and one that receives a TaskB
, (one possible reason is that TaskManager
calls methods that are particular to TaskA
or TaskB
) then you have two options:
- Have two members,
private TaskA taskA
andprivate TaskB taskB
- Have just one member
private Runnable task
, sinceRunnable
is the common ancestor ofTaskA
andTaskB
In the other hand, if you don't really need a constructor for each type of task, you can have just one constructor public TaskManager(Runnable task)
and only one member of type Runnable
like in option 2 previously stated.
It all dependends of whether or not you will need to call methods that are particular to TaskA
or TaskB
. If TaskManager
only really cares about the Runnable
contract then there's no need for two constructors, let alone having to cast an Object
to a Runnable
to run it.
Lastly, if TaskManager
really needs something from the tasks beside the Runnable
functionality, then create a RunnableTask
like this:
public interface RunnableTask extends Runnable {
public int getThis();
public int getThat();
}
public class TaskA implements RunnableTask { ... }
public class TaskB implements RunnableTask { ... }
public class TaskManager {
...
private RunnableTask task;
public TaskManager(RunnableTask task) {
this.task = task;
}
public void startThreadPool() {
threadPool = new ThreadPoolExecutor(...);
threadPool.execute(task);
// do somewthing with task.getThis();
// do somewthing with task.getThat();
}
...
}
Explore related questions
See similar questions with these tags.
private Runnable task;
... and I'm not sure what yourTaskManager
offers above and beyond what theThreadPoolExecutor
offers anyway.