1
\$\begingroup\$

Trying to learn the factory design pattern, came up with some code based on a Shape Factory (found this example here). Is this the right way to implement the factory pattern?

interface Shape{
    void draw();
}

class Rectangle implements Shape{

    @Override
    public void draw() {
        System.out.println(" Drawing A Rectangle!");

    }

}

class Circle implements Shape{

    @Override
    public void draw() {
        System.out.println(" Drawing A cIRCLE!");

    }

}


class Triangle implements Shape{

    @Override
    public void draw() {
        System.out.println(" Drawing A Triangle!");

    }

}


class ShapeFactory{


    public static Shape getShape(String shapeType)
    {
        Shape shape = null;
        switch(shapeType.toUpperCase()){

        case "CIRCLE":
        shape = new Circle();
        break;

        case "RECTANGLE":
            shape = new Rectangle();
            break;

         case "TRIANGLE":
            shape = new Triangle();
            break;
        }

            return shape;
    }


}

public class FactoryExample {

    public static void main(String[] args) {
        Shape s = ShapeFactory.getShape("rectangle");
         s.draw();
         s = ShapeFactory.getShape("triangle");
        s.draw();
    }

}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Try using constants to be passed to the factory for creation. This prevents typos from being introduced, e.g. ShapeFactory.createShape(Shape.TRIANGLE); \$\endgroup\$ Mar 6 '16 at 10:38
5
\$\begingroup\$

You could rewrite getShape as follows:

public static Shape getShape(String shapeType) {
    switch (shapeType.trim().toUpperCase()) {
        case "CIRCLE":
            return new Circle();

        case "RECTANGLE":
            return new Rectangle();

        case "TRIANGLE":
            return new Triangle();

        default:
            return null;
    }
}
\$\endgroup\$
2
\$\begingroup\$

Naming the method getShape(…) suggests that it's going to retrieve the same cached instance with every call. A more appropriate name to convey what the method does would be makeShape(…), or maybe newShape(…). (Unfortunately, you followed a bad example.)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.