5

I want to save Objects of Arc and Line in one ArrayList, then get the intersection of both. The question is how can I cast i and j to its original class. I know that instanceof works but that would be the dirtiest method.

public class Intersection {
    public static boolean intersect(ArrayList<Curve> list1, ArrayList<Curve> list2) {
        for (Curve i : list1) {
            for (Curve j : list2) {
                if (i.intersection(j).length > 0) 
                    return true;
            }
        }
        return false;
    }
}

public abstract class Curve {
    public Point[] intersection(Curve c) {
        return new Point[] {};
    }
}

public class Line extends Curve {
    public Point[] intersection(Line l) {
        // returns intersection Point of this and l
    }

    public Point[] intersection(Arc a) {
        // returns intersection Point(s)
    }
}

public class Arc extends Curve {
    public Point[] intersection(Line l) {
        // return intersection Point(s) of this and l
    }

    public Point[] intersection(Arc a) {
        // returns intersection Point(s)
    }
}

Thanks for your help!

Chetan Kinger
  • 15,069
  • 6
  • 45
  • 82
McTi
  • 91
  • 5
  • I see no problem in using instanceof. Will you have different object types in the same ArrayList or only objects of one type per ArrayList instance? If not, you can use wildcards. – Zoltan Ersek Oct 31 '17 at 14:52
  • 2
    You do realize that `Arc` and `Line` are overloading the parent method and not really overriding it? – Chetan Kinger Oct 31 '17 at 14:54
  • 1
    You are not overriding intersection(Curve) you are just overloading it, note that the code will not work as you expect – Marcos Vasconcelos Oct 31 '17 at 15:00
  • @McTi Follow up question. Do `Line` and `Arc` have additional methods not present in `Curve`? That is, do they have operations different from `Curve` or is `intersection` the only operation? – Chetan Kinger Oct 31 '17 at 15:04
  • @CKing Yes, they have other methods – McTi Oct 31 '17 at 15:07

7 Answers7

2

There are two approaches to tackling such a use-case :


1. Implement Multiple dispatch :

Start by making Curve an interface and add the two overloaded versions of intersect to this interface, thus making them a part of the contract. Next, have the intersection(Curve c) method in each of the sub-classes delegate the call to the appropriate overloaded form. (a.k.a The Visitor pattern)

interface class Curve {
    public Point[] intersection(Curve c);

    public Point[] intersection(Line l);

    public Point[] intersection(Arc c);

}

class Line extends Curve {
    
    public Point[] intersection(Curve c) {
        return c.intersection(this);
    }
    
    @Override
    public Point[] intersection(Line l) {
        System.out.println("line interesection with line");
        return new Point[0];
    }

    @Override
    public Point[] intersection(Arc c) {
        System.out.println("line intersection with arc");
        return new Point[0];
    }

}

class Arc extends Curve {
    
    public Point[] intersection(Curve c) {
        return c.intersection(this);
    }
    @Override
    public Point[] intersection(Line l) {
        System.out.println("arc interesection with line");
        return new Point[0];
    }

    @Override
    public Point[] intersection(Arc c) {
        System.out.println("arc interesection with arc");
        return new Point[0];
    }
}

You can then call your intersection method in the Intersection class without needing any explicit casts :

public class Intersection {
    public static boolean intersect(ArrayList<Curve> list1,
            ArrayList<Curve> list2) {
        for (Curve i : list1) {
            for (Curve j : list2) {
                if (i.intersection(j).length > 0)
                    return true;
            }
        }
        return false;
    }
    
    public static void main(String[] args) {
        Curve line1 = new Line();
        Curve arc1 = new Arc();
        Curve line2 = new Line();
        Curve arc2 = new Arc();
        
        ArrayList<Curve> list1 = new ArrayList<>();
        ArrayList<Curve> list2 = new ArrayList<>();
        list1.add(line1);
        list1.add(arc1);
        list2.add(line2);
        list2.add(arc2);
        
        Intersection.intersect(list1, list2);
    
    }
}

Extras : Take a look at this alternate approach to implementing the Visitor pattern.


2. Make line and curve adhere to the same interface (contract) :

If Line and Arc adhere to the interface of Curve, your code will no longer need the overloaded versions of the intersect method. If we say that a Line is a Curve and an Arc is also a Curve, both these classes should have the same interface as Curve (by interface I mean the list of operations they support). If these classes don't have the same interface as Curve, this is where the problem area lies. The methods present in Curve should be the only methods that should be required by the Line and Arc classes.

There are several strategies to eliminate the need for sub-classes to have methods not present in the superclass :

  • If a subclass requires additional inputs compared to the superclass, provide these inputs via the constructor rather than creating seperate methods that operate on these inputs.
  • If a subclass requires additional behavior not supported by the superclass, support this behavior via composition (read Strategy pattern) rather than adding methods to support additional behavior.

Once you eliminate the need to have specialized methods in the subclass not present in the superclass, your code automatically eliminates the need to have instanceof or type checks. This is in-line with the Liskov substitution principle.


Community
  • 1
  • 1
Chetan Kinger
  • 15,069
  • 6
  • 45
  • 82
1

An alternative would be to use isAssignableFrom method of Class Class. Below is an example:

Exception e = new Exception();
RuntimeException rte = new RuntimeException();
System.out.println(e.getClass().isAssignableFrom(RuntimeException.class));
System.out.println(rte.getClass().isAssignableFrom(Exception.class));
System.out.println(rte.getClass().isAssignableFrom(RuntimeException.class));

Here's the javadoc of isAssignableFrom method and this is what it says:

Determines if the class or interface represented by this Class object is either the same as, or is a superclass or superinterface of, the class or interface represented by the specified Class parameter. It returns true if so; otherwise it returns false. If this Class object represents a primitive type, this method returns true if the specified Class parameter is exactly this Class object; otherwise it returns false.

Darshan Mehta
  • 30,102
  • 11
  • 68
  • 102
1

Since each sub-class already has to know about the other sub-classes (for example, Arc must be aware of the Line class in order to implement Arc and Line intersection), there is nothing wrong with using instanceof.

In each sub-class you can override the base class's public Point[] intersection(Curve c) method and dispatch the implementation to one of the overloaded methods.

For example:

public class Arc extends Curve {    
    @Override
    public Point[] intersection(Curve c) {
        if (c instanceof Line)
            return instersection ((Line) c);
        else if (c instanceof Arc)
            return intersection ((Arc) c);
        else
            return an empty array or null, or throw some exception
    }

    public Point[] intersection(Line l) {
        // return intersection Point(s) of this and l
    }

    public Point[] intersection(Arc a) {
        // returns intersection Point(s)
    }
}

This way you don't have to change anything in your public static boolean intersect(ArrayList<Curve> list1, ArrayList<Curve> list2) method.

Chetan Kinger
  • 15,069
  • 6
  • 45
  • 82
Eran
  • 387,369
  • 54
  • 702
  • 768
  • Sorry, I posted my idea before I saw yours :) – McTi Oct 31 '17 at 15:12
  • @McTi Correct me if I am wrong but you said, *"I know that instanceof works but that would be the dirtiest method."* implying that you are looking for a solution that doesn't use `instanceof`? I have given you two very good options to avoid using `instanceof` in my answer. I am scratching my head wondering how this is the accepted answer? (No offence to the author of this answer. I am trying to make sense out of the OPs post asking for a solution that doesn't use `instanceof` and then accepting an answer that does) – Chetan Kinger Nov 11 '17 at 10:33
  • @Eran I see two issues with this solution 1) The code in the `intersection(Curve c)` method would need to be repeated in all sub-classes which is fine but 2) this can easily become a maintenance nightmare if one has to introduce more subclasses (say Elipse, Parabola,etc) as it would require adding a new `else if` condition and `instanceof` check to all the subclasses. Even if there is no requirement to support anything but a Line or Curve, repeating the `instanceof` checks in even one additional subclass seems like a code smell. – Chetan Kinger Nov 11 '17 at 12:09
0

First consideration: do you need to convert (upcast) i and j from Curve to Arc or Line ?

Please have a look, for example, here:

What's the need to use Upcasting in java?

If you decide you really do need to upcast, unfortunately there is no magic egg - you can't avoid using instanceof to decide the class to upcast to.

You can delegate the responsibility to another class, but basically you can't avoid it.

Sorry!

vikingsteve
  • 38,481
  • 23
  • 112
  • 156
0

Change Curve to an interface. Keep the ArrayList<Curve> the same and instead, extract your intersection method to a separate class, and have it operate on Curves.

You'll need to use the instanceof check there, but your design will be a bit cleaner due to its use of inheritance.

public interface Curve {
...
}

public class Line extends Curve {
...
}

public class Arc extends Curve {
...
}

public class IntersectionUtility {
    public static boolean intersects(ArrayList<Curve> list1, ArrayList<Curve> list2) {
        for (Curve i : list1) {
            for (Curve j : list2) {
                if (i.intersection(j).length > 0) 
                    return true;
            }
        }
        return false;
    }

   public Point[] intersection(Curve a, Curve b) {
      if (a.instanceof(Line.class)) {
        if (b.instanceof(Line.class)) {
           return findIntersection((Line) a, (Line) b); // two Lines
        } else {
           return findIntersection((Line) a, (Arc) b); // a Line and an Arc
        }
      } else {
        if (b.instanceof(Line.class)) {
           return findIntersection((Line) b, (Arc) a); // a Line and an Arc
        } else {
           return findIntersection((Arc) a, (Arc) b); // two Arcs
        }
      }
   }

    public Point[] findIntersection(Line a, Line b) {
        // returns intersection Point of two Lines
    }

    public Point[] findIntersection(Arc a, Arc b) {
        // returns intersection Point(s) of two Arcs
    }

    public Point[] findIntersection(Line a, Arc b) {
        // returns intersection Point(s) of an Line and an Arc
    }
}
Adil B
  • 14,635
  • 11
  • 60
  • 78
0

OK, so one solution I found out would be to use an abstract method in Curve and an if-else chain in subclasses. However I'm not really satisfied with this solultion.

public abstract class Curve {
    public abstract Point[] intersection(Curve c);
}

public class Line extends Curve {
    public Point[] intersection(Curve c) {
        if (c instanceof Line) {
            return this.intersection((Line) c);
        } else if (c instanceof Arc) {
            return this.intersection((Arc) c);
        }
    }

    private Point[] intersection(Line l) {
        // returns intersection Point of this and l
    }

    private Point[] intersection(Arc a) {
        // returns intersection Point(s)
    }
}
McTi
  • 91
  • 5
0

If you dont want to use instanceof then alternative is to use composition for getting the type. The following approach wont be using instanceof and will only use preferred Class.cast operation:

public static class Intersection {

        public static boolean intersect(ArrayList<Curve> list1, ArrayList<Curve> list2) {            
            for (Curve i : list1) {                
                Optional<Line> il = i.get(Line.class);
                Optional<Arc> ia = i.get(Arc.class);

                for (Curve j : list2) {
                    Optional<Line> jl = j.get(Line.class);
                    Optional<Arc> ja = j.get(Arc.class);

                    Point[] intersection = null;

                    if ( il.isPresent() ){
                        if ( jl.isPresent() ) intersection = il.get().intersection( jl.get() );
                        else if ( ja.isPresent() ) intersection = il.get().intersection( ja.get() );
                    }else if ( ia.isPresent() ){
                        if ( jl.isPresent() ) intersection = ia.get().intersection( jl.get() );
                        else if ( ja.isPresent() ) intersection = ia.get().intersection( ja.get() );
                    }    

                    if ( intersection != null && intersection.length > 0 ) return true;
                }
            }
            return false;
        }
    }

    public static abstract class Curve {

        public abstract <T extends Curve> Optional<T> get(Class<T> clazz);

    }

    public static class Line extends Curve {

        public <T extends Curve> Optional<T> get(Class<T> clazz){
            return clazz.equals(Line.class) ? Optional.of( clazz.cast(this) ) : Optional.empty();
        }

        public Point[] intersection(Line l) {
            return new Point[] {};
        }

        public Point[] intersection(Arc a) {
            return new Point[] {};
        }
    }

    public static class Arc extends Curve {

        public <T extends Curve> Optional<T> get(Class<T> clazz){
            return clazz.equals(Arc.class) ? Optional.of( clazz.cast(this) ) : Optional.empty();
        }

        public Point[] intersection(Line l) {
            return new Point[] {};
        }

        public Point[] intersection(Arc a) {
            return new Point[] {};
        }
    } 
tsolakp
  • 5,858
  • 1
  • 22
  • 28