Problems with instanceOf when creating new objects from other objects

Question

I have a class that is responsible for creating Formation objects from Shape objects. Shapes are just what the name says, shapes that are being drawn on canvas (TriangleShape, RectangleShape and so on).

Formations are similar to shapes, but I plan on using them in a different way.

The RectangleShape, for example, looks like this:

public class RectangleShape extends Shape {

public RectangleShape() {
    this(0, 0, 0, 0);
}

public RectangleShape(int x, int y, int width, int height) {
    this.x = x;
    this.y = y;
    this.width = width;
    this.height = height;

    this.nPoints = 4;
}

@Override
public void drawShape(Graphics2D g) {
    Color color = g.getColor();
    fillShape(g, new Color(g.getColor().getRed(), g.getColor().getGreen(), g.getColor().getBlue(), 16));
    g.setColor(color);

    g.drawLine(x, y, x + width, y);
    g.drawLine(x, y, x, y + height);
    g.drawLine(x, y + height, x + width, y + height);
    g.drawLine(x + width, y, x + width, y + height);
}

@Override
public String toString() {
    return "Rectangle";
}

@Override
public Shape createCopy() {
    return new RectangleShape();
}

@Override
public void fillShape(Graphics2D g) {
    xPoints = new int[] {
        x,
        x,
        x + width,
        x + width
    };

    yPoints = new int[] {
        y,
        y + height,
        y + height,
        y
    };

    g.fillPolygon(xPoints, yPoints, nPoints);
}
}

I keep a list of all drawn shapes declared as List<Shape> = new ArrayList<>();.

My problem comes when I need to dynamically create a formation from a shape. The first approach was to have a class with methods like this:

public static TriangleFormation createFormationFrom(TriangleShape shape) {
    // my code here
}

public static RectangleFormation createFormationFrom(RectangleShape shape) {
    // my code here
}

public static PentagonFormation createFormationFrom(PentagonShape shape) {
    // my code here
}

public static HexagonFormation createFormationFrom(HexagonShape shape) {
    // my code here
}

public static OvalFormation createFormationFrom(OvalShape shape) {
    // my code here
}

The problem is when I retrieve the shape from my list, it is of type Shape and I cannot call any of those methods without down-casting the shape to a proper class, which then pulls the question of using instanceOf operator.

Should I merge the Shape and Formation in one class, should I try to implement a Visitor pattern maybe (if so, how would that be done in this case) or is there something else I didn't think of?

I don't see why you dont want to down cast? wont that solve the problem? just to instanceof test first to check if it is the right instance type. — Vegard
– Vegard, Commented May 24, 2013 at 16:55
What about having a generic Formation<S extends Shape> class instead? Of course this might not work if each Formation is fundamentally different. — nitegazer2003
– nitegazer2003, Commented May 24, 2013 at 16:58
Downcasting is an ugly and hard to extend solution. A visitor makes sense if you won't get too many shapes and don't want to support unkown shapes ie. plugins providing additional one. It makes even more sense there is much more type specific logic than you want to put into your class like creating popup menus. Maybe you can even consider a FormationVisitor instead of a FormationFactoryVisitor. However another approach is to have a toFormation() method on each shape — Bernd Ebertz
– Bernd Ebertz, Commented May 24, 2013 at 17:05

kuporific · Accepted Answer · 2013-05-24 17:18:01Z

1

It all depends on how separate you want your Shapes and Formations to be. The simplest solution would be--as you mentioned--to add a Formation createFormation() method to the Shape interface.

But if you're trying to keep Shapes and Formations separate, you'll have to do something more advanced. I would recommend NOT using the visitor pattern, since it is very inflexible.

As an alternative, consider adding a FormationBuilder class:

public interface FormationBuilder
{
    /**
     * Builds a particular Formation implementation from the given shape.
     * Perhaps the shape is passed into the constructor of the Formation,
     * perhaps this method extracts the necessary information to build the
     * Formation...
     */
    Formation build(Shape shape);
}

This can be used with a factory such as this:

public class FormationFactory
{
    private final Map<Class<? extends Shape>, FormationBuilder> builders
            = new HashMap<Class<? extends Shape>, FormationBuilder>();

    public <T extends Shape> void register(
            Class<T> shapeType, FormationBuilder builder);
    {
        builders.put(shapeType, builder);
    }

    public Formation getFormation(Shape shape)
    {
        return builders.get(shape.getClass()).build(shape);
    }
}

But now the question is, where should the Factory be initialized. Whether or not this fits your needs depends on how your code is structured.

answered May 24, 2013 at 17:18

kuporific

10.3k3 gold badges46 silver badges48 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

ioreskovic Over a year ago

Even though I wouldn't mind dwelling in finding the most cleanest solution, I think I'm going to go with toFormation() approach. Ty for all your efforts :)

Collectives™ on Stack Overflow

Problems with instanceOf when creating new objects from other objects

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related