0

At the moment my class looks something like this (very simplified):

I have three classes to describe either Nodes or Ways (from OpenStreetMap):

public abstract class Geometry {
    private String id;

    public Geometry(String id) {
        this.id = id;
    }
}

public class Node extends Geometry {
    private GeoPoint location;

    public Point(String id, GeoPoint location) {
        super(id);
        this.location = location;
    }
    public GeoPoint getLocation() {
        return location;
    }
}


public class Ways extends Geometry {
    private ArrayList <GeoPoint> shape;

    public Point(String id, ArrayList <GeoPoint> shape) {
        super(id);
        this.shape = shape;
    }
    public GeoPoint getShape() {
        return shape;
    }

}

Now I want to iterate through an ArrayList with the class Geometry and use methods from both subclasses:

private void prepareList(ArrayList<Geometry> geometries) {
    for (Geometry m : geometries) {
         if (m.getClass().equals(Node.class)) {
             location = m.getLocation();
         }
         else if (m.getClass().equals(Way.class)) {
             shape = m.getShape();
         }
    }
}

In my solution, I need to make some dummy methods in the class Geometry to access these methods, like public GeoPoint getLocation() { return null; }

My question now is, what is the best way to implement this in Java, without making separate classes (which leads to code duplicates) or writing this "dummy" methods. Is there a better way?

Jaroslav Kadlec
  • 2,505
  • 4
  • 32
  • 43
  • [`instanceof`](http://stackoverflow.com/questions/7313559/what-is-the-instanceof-operator-used-for) is your friend. I did read your question 3 times and I'm sorry but I don't understand it. Could you elaborate (what dummy method are you talking about)? Perhaps you should post the code you have (with dummy methods) –  Apr 14 '14 at 17:34
  • If you're going the route of type-checking the Geometries, you could just typecast it and call the appropriate method. ie: `if (m instanceof Node) { location = ((Node)m).getLocation() }` Though I'm not sure if this is the best design. It would be better to see if you can make an abstract method that both classes implement. – Luuk Apr 14 '14 at 17:52
  • I would avoid instanceof if possible. It's a pretty good indication that you're not writing OO – Amir Afghani Apr 14 '14 at 17:56
  • You could consider using the visitor pattern as described in the answer to this post [link](http://stackoverflow.com/questions/8841577/is-this-use-of-the-instanceof-operator-considered-bad-design?rq=1). I think this is probably the best option. – Luuk Apr 14 '14 at 18:00
  • @Amir I completely agree, this is just essentially what he was doing originally. This is why I linked the Visitor pattern. – Luuk Apr 14 '14 at 18:01
  • Thanks, I will try to implement this with the Visitor Pattern and then write a comment how it worked. – Marinator123 Apr 15 '14 at 09:14
  • @Luuk The typecast really would be a solution here, but I'm also not very convinced that this is good design. At least I can omit the "return null" methods like this. – Marinator123 Apr 15 '14 at 09:26

1 Answers1

0

If I'm understanding your question correctly, you could simply declare an abstract method on Geometry that is implemented by the Node and Ways sub-classes.

public abstract class Geometry {
    private String id;

    public Geometry(String id) {
        this.id = id;
    }

    public abstract GeoPoint getLocation();
}

or consider making Geometry an interface. Something like:

public interface Traversable { 

    public String getId();

    public GeoPoint getLocation();

}

and implement this interface on Ways and Nodes. I would prefer the latter approach, as a class can implement multiple interfaces, but can only extend one super class.

EDIT: It's a little clearer to me now that you are not returning the same thing from Ways and Nodes. You could:

  • Wrap the results in a class that can deal with a single GeoPoint vs a List of GeoPoints
  • Re-work this code to perform whatever operation they Way or Node needs to perform inside of the sub-class.
  • Use a List of GeoPoints as the return type of the method, and a node only has one element that is returned vs many points returned by a Way
Amir Afghani
  • 37,814
  • 16
  • 84
  • 124
  • Ways doesn't uses getLocation, it uses getShape which returns an ArrayList of GeoPoints. I was thinking this too, but that won't work in this case. – Luuk Apr 14 '14 at 17:41
  • I see your point Luuk. It sounds like this should be an operation they each perform. Or he needs a class that wraps the return value of the interface method. – Amir Afghani Apr 14 '14 at 17:43
  • Thanks for the answers. Maybe for the better understanding: I'm receiving an XML-File from the OpenStreetMap Server with Ways and Nodes, and parse through this file to find all relevant Geometries and save them in the ArrayList . The Way and Node class share about 60% of their properties, that's why I use a parent class geometries. Yet, the other part (methods in the subclasses such as getLocation) of the class isn't very useful for the classes. To access this methods when parsing through the geometry list, I am forced to write the "dummy" methods in the parent class. – Marinator123 Apr 15 '14 at 09:09