0

I am having some trouble applying Factory Pattern.

I have a class that I usually call as Product($modelNumber, $wheelCount). But in a part of legacy code that I am refactoring, I do not have $modelNumber, and only have $productID, where the link between {$modelNumber, $productID} is in the database (or in my case I can hardcode it, as I only have a select few products at the moment).

I need to be able to create my class using $productId, but how?

Using Procedural ways I would have a function that does the lookup, and I would put that function in a file, and include that file anywhere where I need to do the lookup. Thus do this:

$modelNumber = modelLookup($productId)
Product($modelNumber, $wheelCount);

But how do I do it using Object Oriented way?

Note: I have posted a more detailed situation here: https://softwareengineering.stackexchange.com/q/233518/119333 and this is where Factory pattern (and other patterns, like interfaces and function pointer passing) were suggested conceptually, but I hit a wall when trying to implement them in PHP. It kind of seems like a simple question, but I think there are several ways to do it and I am a bit lost as to how. And so I need some help.

Community
  • 1
  • 1
Dennis
  • 7,907
  • 11
  • 65
  • 115

2 Answers2

1

I provided a conceptual answer to your SRP problem on Programmers Exchange but I think I can demonstrate it here.

What you basically want is some other object that will do the work to get you the model number of given product ID.

class ProductModelNumberProvider {
    public function findByProductId($productId) {
        // The lookup logic...
    }
}

Your factory should provide a setter constructor so it can make use of this object internally to lookup the model number if needed. So basically you will end up with a ProductFactory similar to this.

class ProductFactory {
    private $productModelNumberProvider;
    
    public function __construct(ProductModelNumberProvider $productModelNumberProvider) {
        $this->productModelNumberProvider = $productModelNumberProvider;
    }

    public function getProductByIdAndWheels($productId, $wheels) {
        $modelNumber = $this->productModelNumberProvider($productId);
        return $this->getProductByModelNumberAndWheels($modelNumber, $wheels);
    }

    public function getProductByModelNumberAndWheels($modelNumber, $wheels) {
        // Do your magic here...
        return $product;
    }
}

EDIT

On second thought the setter is not the best approach since having a ProductModelNumberProvider instance is mandatory. That is why I moved it to have it injected through the constructor instead.

Community
  • 1
  • 1
Bart
  • 17,070
  • 5
  • 61
  • 80
  • so then when you have $modelNumber, you call the class directly, and when you have only $prodiuctId, you call the Factory, correct? – Dennis Mar 25 '14 at 19:35
  • also, when I do have $productID, I see I need to first instantiate the Provider class, then pass it to the Factory. Something like `new ProductFactory(new ProductModelNumberProvider($productID))`. I know it's DI, but still it looks a bit convoluted – Dennis Mar 25 '14 at 19:46
  • You best always rely on the factory to create the products. It gives a clear interface and location for constructing them. The approach demonstrated is not convoluted at all. You basically invented your own interface in your code sample which is different from mine. – Bart Mar 25 '14 at 20:00
  • The same approach could be taken to encapsulate the calculations as well. Ideally the factory is constructed only once when your application is bootstrapped. It should not be constructed for every single product you want to construct. – Bart Mar 25 '14 at 20:10
  • oops that was wrong.. it is supposed to be `new ProductFactory(new ProductModelNumberProvider())->getProductByIdAndWheels(($productId, $wheels));`. But how would you use this ProductFactory to create a product by the model number? – Dennis Mar 25 '14 at 20:14
  • use the other method :-) `getProductByModelNumberAndWheels($modelNumber, $wheels)` – Bart Mar 25 '14 at 20:23
  • ah so `ProductModelNumberProvider()` just stays unused in that case. – Dennis Mar 25 '14 at 20:26
  • While technically correct, ... sadly for me the readability suffers. Compare to a more readable `Product::fromProductId($productId, $wheels);` using something like this: http://stackoverflow.com/a/2175213/2883328 – Dennis Mar 25 '14 at 21:40
  • Dennis, the naming of the methods are not set in stone. Feel free to rename them to whatever you like better. It's the concept that counts. – Bart Mar 26 '14 at 04:45
  • Don't use the product class as the factory by the way since you are goung to mix concerns again (where will the provider go?) You are sacrificing all benefits of my answer just to have everything inside a single object again. If you are concerned about OOP and SRP then please follow that path and do not sidestep whenever you feel like it so you can save a few lines of code. Also think about the testability of the code which will become harder with every responsibility you give to a object. – Bart Mar 26 '14 at 05:54
  • Thanks, that make sense. I had a question on the factory though -- why do we need it? Why not for example instantiate the provider first, use it to convert productId to modelNumber, and call the class directly with the modelNumber? It is still testable and still object oriented, just without the factory being "in the way". aka what is the benefit of the factory really? – Dennis Mar 26 '14 at 13:58
  • reading about Factory Pattern, it hides the actual object that is being created, so that that object can be later swapped by something else or further customized without possibly having to change the Factory-calling code. In my case though I think that using the Provider is a great idea and I will go with that. As for Factory I do not see it as something that I must have at this time. It will just add code with no significant benefit. – Dennis Mar 26 '14 at 14:10
  • If later I have multiple types of products maybe there is more of a benefit to the Factory. But still I do not see a clear benefit to use the Factory over just instantiating Products directly. I guess I'm using YAGNI – Dennis Mar 26 '14 at 14:14
  • actually.. if I do have a lot of products it might be easier to say `$factory->createProductXfromY()` but still no direct benefit there as I have to specify what product I want to create I might as well just use the `ProductX()` directly. It gives me food for thought though on when I do need a Factory I can put it in. – Dennis Mar 26 '14 at 14:33
  • I don't know how many times you are creating a product from it's `productId` but when you create the product more then once the benefit of the factory becomes more clear. A factory hides the construction logic. If you have to lookup the model number all over the place just to create a product you will end up with a lot of duplicate code. The factory gives another benefit as a DRY approach to the problem. – Bart Mar 26 '14 at 15:46
  • right now I call it in two places, but I think it depends on how much code you have to construct to get the object. i.e. right now I have to construct the Provider everywhere where I call the factory. Provider can be considered part of construction code and hence part of duplication that I will be putting everywhere as well. .. UPDATE: Oh wait... maybe I can construct provider once only and reuse it in several places. So then what you said makes sense, if I can instantiate Provider once somewhere and reuse it – Dennis Mar 26 '14 at 17:40
0

I can think of something like this:

$factory = new ProductBuilder();
$factory->buildFromProductId($productId, $wheelCount); //uses modelLookup() internally
$factory->buildFromModelNumber($modelNumber, $wheelCount); //just returns Product()

It is basically creating a class on top of the procedural function, but it does separate the logic of creating the class separately from looking up the mapping.

Dennis
  • 7,907
  • 11
  • 65
  • 115