how can I replace instanceof in this case?

Question

I'm trying to compare compareCriteria. Simple ones like 'between' and 'inArray' or 'greaterThan'. I use polymorphism for these classes. One method they share from the compareCriteria interface is 'matchCompareCriteria'.

What I'm trying to avoid is having each class check for the type of compareCriteria they should match against. Eg the inArray object will check if matchCompareCriteria is passed an inArray object, if not it will return false, in the case it is it knows how to compare.

Maybe instanceof is perfectly legitimate in this case (the objects know about themselves) but still I'm looking at possible ways to avoid it. Any ideas?

pseudo-code example:

betweenXandY = create new between class(x, y)
greaterThanZ = create new greaterThan class(z)
greaterThanZ.matchCompareCriteria(betweenXandY)

if X and Y are greater than Z it will return true.

edit:

1) instanceof is what I see, for now, as needed in the matchCompareCriteria method. I'd like to get rid of it

2) matchCompareCritera checks if a compareCriteria is contained by another. If all possible values of one is contained by the other it returns true. For many combinations of compareCriteria it doens't even make sense to compare them so they return false (like betweenAlfa and betweenNum would be incompatible).

Answer 1

You need to create a super class or interface called Criteria. Then each concrete sub class will implement the Criteria interface. between, greaterthan etc are criterias.

the Criteria class will specify the matchCompareCriteria method which accepts a Criteria. The actual logic will reside in the sub classes.

You are looking for either the Strategy Design pattern or the Template Design PAttern.

Answer 2

Since you reference instanceof, I'm assuming we're working in Java here. This might let you make use of overloading. Consider an interface called SomeInterface, which has a single method:

public interface SomeInterface {
    public boolean test (SomeInterface s);
}

Now, we define two (cleverly named) classes that implement SomeInterface: Some1 and Some2. Some2 is boring: test always returns false. But Some1 overrides the test function when given a Some2:

public class Some1 implements SomeInterface {
    public boolean test (SomeInterface s) {
        return false;
    }

    public boolean test (Some2 s) {
        return true;
    }
}

This lets us avoid having line after line of if statements to do type checking. But there is a caveat. Consider this code:

Some1 s1 = new Some1 ();
Some2 s2 = new Some2 ();
SomeInterface inter = new Some2 ();

System.out.println(s1.test(s2));     // true
System.out.println(s2.test(s1));     // false
System.out.println(s1.test(inter));  // false

See that third test? Even though inter is of type Some2, it is treated as a SomeInterface instead. Overload resolution is determined at compile-time in Java, which could make it completely useless for you.

That puts you back at square one: using instanceof (which is evaluated at run-time). Even if you do it that way, it's still a bad design. Each of your classes has to know about all of the others. Should you decide to add another, you have to go back to all existing ones to add in functionality to handle the new class. This gets horribly unmaintainable in a hurry, which is a good sign that the design is bad.

A redesign is in order, but without a lot more information, I can't give you a particluarly good push in the right direction.

Answer 3

If I understand well, your method relies on type checking. That is quite hard to avoid, and polymorphism fails at solving the problem. From your example, inArray needs to check the type of the parameter because the behaviour of the method depends on this. You can't do that by polymorphism, meaning you can't put a polymorphic method on your classes to handle this case. That is because your matchCompareCriteria depends on the type of the parameter rather than on its behaviour.

The rule of not using instanceof is valid when you check the type of an object to chose what behaviour to have. Clearly, that behaviour belongs on the different objects whose type you check. But in this case, the behaviour of your object depends on the type of the object you are passed in and belongs on the calling object, not on the called ones as before. The case is similar to when you override equals(). You do a type check so that the object passed in is the same type as this object and then implement your behaviour: if the test fails, return false; otherwise, do the equality tests.

Conclusion: using instanceof is ok in this case.

Here is a longer article from Steve Yegge, which explains better, I think, using a simple and straightforward example. I think this maps great to your problem.

Remember: Polymorphism is good, except when it's not. :)

Answer 4

The problem you are describing is called double dispatch. The name comes from the fact that you need to decide which bit of code to execute (dispatch) based on the types of two objects (hence: double).

Normally in OO there is single dispatch - calling a method on an object causes that object's implementation of the method to execute.

In your case, you have two objects, and the implementation to be executed depends on the types of both objects. Fundamentally, there is a coupling implied by this which "feels wrong" when you've previously only dealt with standard OO situations. But it's not really wrong - it's just slightly outside the problem domain of what the basic features of OO are directly suited to solving.

If you're using a dynamic language (or a static-typed language with reflection, which is dynamic enough for this purpose) you can implement this with a dispatcher method in a base class. In pseudo-code:

class OperatorBase
{
    bool matchCompareCriteria(var other)
    {
        var comparisonMethod = this.GetMethod("matchCompareCriteria" + other.TypeName);
        if (comparisonMethod == null)
            return false;

        return comparisonMethod(other);
    }
}

Here I'm imagining that the language has a built-in method in every class called GetMethod that allows me to look up a method by name, and also a TypeName property on every object that gets me the name of the type of the object. So if the other class is a GreaterThan, and the derived class has a method called matchCompareCriteriaGreaterThan, we will call that method:

class SomeOperator : Base
{
    bool matchCompareCriteriaGreaterThan(var other)
    {
        // 'other' is definitely a GreaterThan, no need to check
    }
}

So you just have to write a method with the correct name and the dispatch occurs.

In a statically typed language that supports method overloading by argument type, we can avoid having to invent a concatenated naming convention - for example, here it is in C#:

class OperatorBase
{
    public bool CompareWith(object other)
    {
        var compare = GetType().GetMethod("CompareWithType", new[] { other.GetType() });
        if (compare == null)
            return false;

        return (bool)compare.Invoke(this, new[] { other });
    }
}

class GreaterThan : OperatorBase { }
class LessThan : OperatorBase { }

class WithinRange : OperatorBase
{
    // Just write whatever versions of CompareWithType you need.

    public bool CompareWithType(GreaterThan gt)
    {
        return true;
    }

    public bool CompareWithType(LessThan gt)
    {
        return true;
    }
}

class Program
{
    static void Main(string[] args)
    {
        GreaterThan gt = new GreaterThan();
        WithinRange wr = new WithinRange();

        Console.WriteLine(wr.CompareWith(gt));
    }
}

If you were to add a new type to your model, you would need to look at every previous type and ask yourself if they need to interact with the new type in some way. Consequently every type has to define a way of interacting with every other type - even if the interaction is some really simple default (such as "do nothing except return true"). Even that simple default represents a deliberate choice you have to make. This is disguised by the convenience of not having to explicitly write any code for the most common case.

Therefore, it may make more sense to capture the relationships between all the types in an external table, instead of scattering it around all the objects. The value of centralising it will be that you can instantly see whether you have missed any important interactions between the types.

So you could have a dictionary/map/hashtable (whatever it's called in your language) that maps a type to another dictionary. The second dictionary maps a second type to the right comparison function for those two types. The general CompareWith function would use that data structure to look up the right comparison function to call.

Which approach is right will depend on how many types you're likely to end up with in your model.

Answer 5

The Smalltalk approach would be to introduce more layers to the hierarchy. So between and greaterThan would be subclasses of rangedCompareCriteria (or something), and rangeCompareCriteria::matchCompareCriteria would return true when asked if two instances of itself were comparable.

Speaking of which, you probably want to rename "matchCompareCriteria" to something that expresses the intent a bit better.