2

I am concerned about whether or not I am using FactoryMethod correctly.

Background:

I am refactoring legacy code and I have identified a pattern: there is a God Object that is created in memory, and later various parts of it is saved to many different tables. For example, various parts are called Selection, Design, Plot.

What I've done

I created a SelectionPersisterFactory that returns a SelectionPersister object, which is responsible for saving Selection to database.

Next step would be to create DesignPersisterFactory that returns DesignPersister, which saves Design to database.

But then I stopped myself and ask:

Question

Can I create a single PersisterFactory and have that return SelectionPersister, DesignPersister, PlotPersister, etc? from one single factory?

Pro/Con

  • "The Pro" will be that all of them are using the same repository and I can initialize that repository one time, instead of doing it for every new type of Persister

  • "The Con" is that I will be tasking the factory with creating different types of Persisters, but I am not sure if it's a necessarily a bad thing. i.e. I am not sure whether or not I'll be breaking SRP.

To clarify my title, "similar purpose" - objects are meant to persist something, "different in focus" - the persister objects focus on different parts of God object.

Goal / Use Case

Current situation is that I have the god object containing everything generated during "customer activity". That object is an in-memory object. It also permeates a lot of the codebase and is impractical to change on a moment's notice. It is not always nicely separated into its own distinct sub-components, although it tries.

The task at hand is "Save this session of customer activity (aka the god object) into the database". That includes saving:

  • User-Selected Items to table items
  • Plot generated from above items into table plot
  • Design object data from above items, into table design
  • and so on (just 1-2 more)

Why you think you need all these different persisters?

Different parts draw from different sections of the god object. Originally it was all procedural code just doing SQL statements doing all of the above DB saving, spanning 500+ some lines. I wanted to break it up into more manageable pieces, and came up with an idea "Let's separate saving different parts of god-object into separate methods". Since they are being persisted into the database, why not call them Persisters.

it would be worth to know if you have only a small fixed number of parts class

Yes I have a fixed number of parts classes, say 5 or 6. Different "customer activity" generates slightly different needs. So some sessions may have the need to persist 1 or 2 parts, some might need all 5. I do not anticipate the need to add more part classes, but rather aim to keep code simple.

Improve this question
6
  • 3
    it's called an "Abstract Factory."
    – Robert Harvey
    Jun 21, 2016 at 16:00
  • @RobertHarvey: not really (at least, not by "the book"). "Abstract factory pattern", to my understanding, means still having different SelectionPersisterFactory, DesignPersisterFactory", ... objects, but they are derived from a common PersisterFactory` base class.
    – Doc Brown
    Jun 21, 2016 at 18:48
  • @DocBrown: Frankly, this smells of "clearly there's a better solution available." I'm not particularly concerned about SRP if breaking it slightly avoids the FactoryFactoryFactory model. Even a Generic Repository would be more favorable.
    – Robert Harvey
    Jun 21, 2016 at 19:18
  • @RobertHarvey: absolutely. But the AbstractFactory pattern, as described by the GOF, AFAIK is an example of that "FactoryFactoryFactory" model, as you called it. I guess the OP should ask himself first if he really needs an own xyzPersister class for each "parts" class, or if he cannot create a more generic "Persister" class, using reflection techniques. Dennis, can you tell us more about why you think you need all these different persisters?
    – Doc Brown
    Jun 21, 2016 at 19:20
  • ... and additionally, it would be worth to know if you have only a small fixed number of parts class (like the 3 you mentioned), or if there are more and you need the factory approach to be extendable when new parts will be introduced, without touching the code of the existing classes (which is called Open/Close principle)?
    – Doc Brown
    Jun 21, 2016 at 19:24

2 Answers 2

Reset to default
2

If you have a small, fixed number of different part types which will be seldom extended, you can keep this pragmatic and give your PersisterFactory a handful of different methods like CreateDesignPersister, CreatePlotPersister or CreateSelectionPersister. If each of those classes contains only a single persisting method, even that maybe overengineering, a generic "Persister" with three methods Save(Design d), Save(Selection s), SavePlot(Plot p) might be fine.

However, if your situation looks differently, and you have many different part types, and you want that list to be easily extendable, you might consider to switch to a more sophisticated design. First, consider if you really need different "Persister" logic for each part type, or if the persisting part cannot be implemented in a more generic fashion, by some kind of GenericPersister class which uses reflection (assumed you use a programming language which supports this) to persist whatever you pass to it.

If that is not possible or not worth the generalization effort, next thing you can try is to have a Persister class which uses the strategy pattern, providing a DesignPersisterStrategy, SelectionPersisterStrategy, and so on. There will be only one Persister object, holding the one and only one reference to your repository (or whatever resources you want to reuse). It will also provide a public Save method which will choose the correct strategy according to the type of the object passed, and then uses it to save the object correctly. So you reduce the number of needed classes from "3 * N" to "2 * N+1" (where N is the number of part types), without violating the SRP, and still sharing common resources.

Improve this answer
4
  • Thanks. your first sentence is what I think I'm doing already. The generic Persister (your second sentence) may be a fine idea but in my case there is a bit of logic that some parts have and some don't. That said, having separate save methods for each may be fine, but I wanted to add a bit of separation by creating different class files for each part to be persisted. For GenericPersister, I use PHP that supports reflection although I don't quite have an image in my mind how that works exactly (Granted I am not sure if it'll worth the effort, despite having peaked my curiosity)
    – Dennis
    Jun 21, 2016 at 20:22
  • ... I like the strategy pattern ..although I hope implementing it won't overly-complicate things. I'll have to see how this plays out a bit.
    – Dennis
    Jun 21, 2016 at 20:23
  • also to add ..the god object I have now doesn't (easily) let me define things like Design, Selection, Plot. I would have to either create a "converter" class to deal with the conversion, or like now, the way conversion is done now is implicit. i.e. (SelectionPersister())->save($godObject), (PlotPersister())->save($godObject), and direct assignment is done inside, where each persister's save method knows what parts of god object to access, and which to leave behind.
    – Dennis
    Jun 21, 2016 at 20:30
  • @Dennis: the strategy pattern gives you one strategy class per "part" class. Your original suggestion uses a special factory class and a special persister class for each "part" class, so the strategy approach is most probably less complicated than having a factory + a persister for each part.
    – Doc Brown
    Jun 21, 2016 at 20:37
1

What you are decribing is a common design pattern, and, as @Robert commented, its name is abstract factory remember SRP means, that a entity must have only one reason to change not that it must only do one thing at all, or return only one type of result. So no, you are not breaking SRP.

Improve this answer
2
  • 1
    See my comment below Robert Harvey's comment - not everything which looks like an abstract factory is one.
    – Doc Brown
    Jun 21, 2016 at 19:02
  • @Doc Brown, I agree, it is not strictly an abstract factory, however, grouping factories is not unseen or uncommon and doesn't break SRP, I'd admit we used the name abstract factory with too much liberty and I will research and edit my answer, thanks for the clarification.
    – Arturo Montaño
    Jun 21, 2016 at 19:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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