24

Why does this ShapeFactory use conditional statements to determine what object to instantiate. Dont we have to modify ShapeFactory if we want to add other classes in the future? Why doesnt this violate the open closed principle?

Factory Pattern Design

ShapeFactory Design

5
  • 2
    Which “factory pattern” are you referring to precisely? In general, a factory is any object or method that serves to instantiate an object. Then there are specific variations of this general idea such as the Abstract Factory Pattern, where each factory instance represents a specific palette of choices – usually managed via subclassing rather than conditionals.
    – amon
    Nov 17, 2015 at 10:34
  • 3
    Thanks for that info, this clears up a lot. That is definitively an example of a factory pattern, but not of the Abstract Factory Pattern commonly associated with factory patterns. The code in that article is quite questionable, and I would not expect to see anything like that in real code.
    – amon
    Nov 17, 2015 at 18:38
  • @ArmonSafai: You're linking this blog post a lot, but you're not really explaining why. Are we all somehow ignorant of the pattern? We have Google too, just like you. Nov 17, 2015 at 19:00
  • 1
    @RobertHarvey I am linking this blog post to show how the factory pattern in that page is using conditionals Nov 17, 2015 at 19:53

7 Answers 7

28

The conventional object-oriented wisdom is to avoid if statements and replace them with dynamic dispatch of overridden methods in subclasses of an abstract class. So far, so good.

But the point of the factory pattern is to relieve you from having to know about the individual subclasses and work only with the abstract superclass. The idea is that the factory knows better than you which specific class to instantiate, and you will be better off working only with the methods publishd by the super class. This is often true and a valuable pattern.

Therefore, there is no way that writing a factory class can forego the if statements. It would shift the burden of choosing a specific class to the caller, which is exactly what the pattern is supposed to avoid. Not all principles are absolute (in fact, no principle is absolute), and if you use this pattern you'd assume that the benefit from it is greater than the benefit of not using an if.

7
  • 3
    It is perfectly possible to create a factory pattern without lots of ifs. See @BЈовић's answer for a simple example of how to achieve this. Downvoted.
    – David Arno
    Nov 17, 2015 at 10:40
  • 12
    @DavidArno Naturally, there are different ways to choose the concrete class. Service Locator is one, a configurable IoC Container is another. Those are just implementation details; they don't detract from Killian's main message, which is that the Factory relieves the caller from having to decide which concrete class to instantiate. Don't get mired in the details. Nov 17, 2015 at 18:53
  • 1
    Terrific statement that does in no way answer the question. Jul 23, 2019 at 5:06
  • 1
    @R.Schmitz I think you are incorrect in your assumption. I think many people missed this question from the OP: "Dont we have to modify ShapeFactory if we want to add other classes in the future?" CLEARLY, the OP is confused thinking that this pattern violates OCP because to add new functionality, you have to modify existing code. The correct answer to this question can be found in my answer. The short answer: You leave that code alone and you apply Abstract Factory pattern to EXTEND (not modify) your existing functionality. Because of this Kilian's answer DOES NOT address the question.
    – hfontanez
    Jul 23, 2019 at 15:41
8

The example probably uses a conditional statement because it is the simplest. A more complex implementation might use a map or configuration or (if you want to be really fancy) some kind of registry where the classes can register themselves. However there is nothing wrong with using a conditional if the number of classes is small and changes infrequently.

Extending the conditional to add support for a new subclass in the future would indeed strictly speaking be a violation of the open/closed principle. The "correct" solution would be to create a new factory with the same interface. That said, adherence to the O/C principle should always be weighed against other design principles like KISS and YAGNI.

That said, the code displayed is clearly example code which is designed to show the concept of a factory and nothing else. E.g. it is really bad style to return null as the example does, but more elaborate error handling would just obscure the point. Example code is not production quality code, any you shouldn't expect it to be.

4
6

The pattern itself doesn't violate the Open/Closed Principle (OCP). However, we violate the OCP when we use the pattern incorrectly.

The simple answer to this question is as follows:

  1. Create your base functionality using Factory Method Pattern.
  2. EXTEND your functionality by using the Abstract Factory Pattern

In the provided example, your base functionality supports three shapes: Circle, Rectangle, and Square. Suppose you need to support Triangle, Pentagon, and Hexagon in the future. To do this WITHOUT violating OCP, you must create an additional factory to support your new shapes (let's called AdvancedShapeFactory) and then use AbstractFactory to decide what factory you need to create in order to create whatever shapes you need.

1
  • I much prefer the self-registering factories solution (in the absence of a true configurable IoC container which is best), since otherwise what we get from your proposal is basically factories of factories, and that's when things get just overly complex.
    – Nom1fan
    Jan 25, 2020 at 12:59
1

If you're talking about the Abstract Factory pattern, the decision making often isn't in the Factory itself but in application code. It's that code that chooses what concrete factory to instantiate and pass around to the client code that will use objects produced by the Factory. See the end of the Java example here : https://en.wikipedia.org/wiki/Abstract_factory_pattern

Decision making not necessarily implies if statements. It could read the concrete Factory type from a config file, derive it from a map structure, etc.

3
  • If I, the caller, am making the decision which concrete class to instantiate, then why am I bothering with an Abstract Factory? Nov 17, 2015 at 19:00
  • Please define "the caller". As I describe in my answer, there is the global application code and then there's the code that needs to spawn objects using a Factory. While the latter indeed needs to be unaware of the concrete class to instantiate, some other contextual code has to know about it and new it up... Nov 18, 2015 at 8:30
1

If you think about the Open-Close at class level with this factory you are making other class in your system Open-Close, for example if you have other class that take one Shape and calculate the area (typical example) this class is OpenClose because it can calculate the area for new types of shapes without modification. Then you have another class that draws the shape, another class that take N shapes and return the bigger one and you can think in general that the other classes in your system that deals with shapes are Open-Close (at least about shapes). Looking at the design the factory enables the rest of the system to be Open-Close and off course the factory itself ITS NOT Open-Close.

Off course you can make this factory open-close too, via some kind of dynamic loading and your entire system can be Open-Close (you can add new shapes dropping some jar in the classpath for example). You need to evaluate is this extra complexity is worth depending on the system your are building, not all systems need pluggable features and not all the system need to be completely Open-Close.

0

The open-closed principle, as the Liskov substitution principle, applies to class trees, to inheritance hierarchies. In your example, the factory class is not in the family tree of the classes it instantiates so it cannot violate these rules. There would be a violation if your GetShape (or more aptly named, CreateShape) were implemented in the Shape base class.

-2

It all depends how you implement it. You can use std::map to hold function pointers to functions creating objects. Then the open/close principle is not violated. Or switch/case.

Anyway, if you do not like the factory pattern, you can always use dependency injection.

6
  • 6
    How is switch/case better than conditionals? Using a map/dict/table to represent code as data is good, if you actually need a registry of different implementations – e.g. in some DI container implementations. But having different callbacks of the same type is not necessary for most factories! I don't quite get why you are suggesting that. Also, many DI containers are implemented in terms of factory objects, so suggesting to use DI instead of factories seems a bit circular.
    – amon
    Nov 17, 2015 at 11:23
  • 1
    @amon I meant to use other types of DI - not the factory. Nov 17, 2015 at 12:34
  • 1
    How's your factory going to decide which pointer to use? Eventually you have to make a decision. Nov 18, 2015 at 5:11
  • @ArmonSafai ??? Nov 18, 2015 at 6:19

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