Is it a good practice if using Object when a method can accept more than one type of object?

Question

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?

`private Runnable task;`... and I'm not sure what your `TaskManager` offers above and beyond what the `ThreadPoolExecutor` offers anyway. — , Mar 07 '17 at 01:37

user949300 · Accepted Answer · 2017-03-07T16:43:16.960

9

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.

edited Mar 07 '17 at 16:43

answered Mar 07 '17 at 04:03

user949300

8,679
2
26
35

Changing it to a `Runnable`, yes. Renaming it?... Very debatable. – svidgen Mar 07 '17 at 16:31
Valid point, edited the answer. – user949300 Mar 07 '17 at 16:42

score 2 · Answer 2 · answered Mar 07 '17 at 14:33

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 and private TaskB taskB
Have just one member private Runnable task, since Runnable is the common ancestor of TaskA and TaskB

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();
    }
    ...
}

Is it a good practice if using Object when a method can accept more than one type of object?

2 Answers2