Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Am I overdoing it with my Factory Method?

Part of our core product is a website CMS which makes use of various page widgets. These widgets are responsible for displaying content, listing products, handling event registration, etc. Each widget is represented by class which derives from the base widget class. When rendering a page the server grabs the page's widget from the database and then creates an instance of the correct class. The factory method right?

Private Function WidgetFactory(typeId)
    Dim oWidget
    Select Case typeId
        Case widgetType.ContentBlock
            Set oWidget = New ContentWidget
        Case widgetType.Registration
            Set oWidget = New RegistrationWidget
        Case widgetType.DocumentList
            Set oWidget = New DocumentListWidget
        Case widgetType.DocumentDisplay
    End Select
    Set WidgetFactory = oWidget
End Function

Anyways, this is all fine but as time has gone on the number of types of widgets has increased to around 50 meaning the factory method is rather long. Every time I create a new type of widget I go to add another couple of lines to the method and a little alarm rings in my head that maybe this isn't the best way to do things. I tend to just ignore that alarm but it's getting louder.

So, am I doing it wrong? Is there a better way to handle this scenario?

like image 740
jammus Avatar asked Dec 17 '09 09:12

jammus


1 Answers

I think the question you should ask yourself is: Why am I using a Factory method here?

If the answer is "because of A", and A is a good reason, then continue doing it, even if it means some extra code. If the answer is "I don't know; because I've heard that you are supposed to do it this way?" then you should reconsider.

Let's go over the standard reasons for using factories. Here's what Wikipedia says about the Factory method pattern:

[...], it deals with the problem of creating objects (products) without specifying the exact class of object that will be created. The factory method design pattern handles this problem by defining a separate method for creating the objects, whose subclasses can then override to specify the derived type of product that will be created.

Since your WidgetFactory is Private, this is obviously not the reason why you use this pattern. What about the "Factory pattern" itself (independent of whether you implement it using a Factory method or an abstract class)? Again, Wikipedia says:

Use the factory pattern when:

  • The creation of the object precludes reuse without significantly duplicating code.
  • The creation of the object requires access to information or resources not appropriate to contain within the composing object.
  • The lifetime management of created objects needs to be centralised to ensure consistent behavior.

From your sample code, it does not look like any of this matches your need. So, the question (which only you can answer) is: (1) How likely is it that you will need the features of a centralized Factory for your widgets in the future and (2) how costly is it to change everything back to a Factory approach if you need it in the future? If both are low, you can safely drop the Factory method for the time being.


EDIT: Let me get back to your special case after this generic elaboration: Usually, it's a = new XyzWidget() vs. a = WidgetFactory.Create(WidgetType.Xyz). In your case, however, you have some (numeric?) typeId from a database. As Mark correctly wrote, you need to have this typeId -> className map somewhere.

So, in that case, the good reason for using a factory method could be: "I need some kind of huge ConvertWidgetTypeIdToClassName select-case-statement anyway, so using a factory method takes no additional code plus it provides the factory method advantages for free, if I should ever need them."

As an alternative, you could store the class name of the widget in the database (you probably already have some WidgetType table with primary key typeId anyway, right?) and create the class using reflection (if your language allows for this type of thing). This has a lot of advantages (e.g. you could drop in DLLs with new widgets and don't have to change your core CMS code) but also disadvantages (e.g. "magic string" in your database which is not checked at compile time; possible code injection, depending on who has access to that table).

like image 143
Heinzi Avatar answered Oct 24 '22 22:10

Heinzi