Is It Bad Practice to Check Class Instances by Interface in Java?

designjavaprogramming practicesreflection

I have a class (Timer) with an array list of Timable objects. Timeable is an interface. There is some specific functionality that I need for the Trigger class (implements Timable), which has a reference called target. A lot of methods need to search through the Timer array for Trigger objects with a certain target.

What I did was a function like this:

public Timable[] findObjectsWithTarget(Cue target) {
    ArrayList<Timable> result = new ArrayList<Timable>();
    for (Timable timed : fireable) { //fireable is the array (actually a HashSet) of Timed objects
        if (timed instanceof Trigger && ((Trigger) timed).getTarget() == target)
            result.add(timed);
    }
    return (Timable[]) result.toArray();
}

This feels like a bad practice. The Timer class is now dependent on the Trigger class (sort of) and are no longer generalized.

So, my two questions:

1) Is it actually a bad practice? Why or why not?

2) What's a better way if it is a bad practice?

Best Answer

Yes this is indeed a bad practice. You are using an interface to abstract your code from the implementation. When you work with the interface and access the implementation (via casting) you are violating the interface definition.

To solve this kind of problem extend your interface by another method, that you would access from within your code.

public Timable[] findObjectsWithTarget(Cue target) {
    ArrayList<Timable> result = new ArrayList<Timable>();
    for (Timable timed : fireable) { //fireable is the array (actually a HashSet) of Timed objects
        if (timed.hasTarget(target))
            result.add(timed);
    }
    return (Timable[]) result.toArray();
}

You have to implement that method also in Timerclass but you can return false per default.

Related Topic