4

I have a class named Product, which takes a product model number and number of wheels for that product as part of its creation information, and then it instantiates the product by doing a computation on the number of wheels and on various base parameters

For example, Product(5, 22) creates a model 5 product with 22 wheels on it, where that particular product's weight and dimensions are computed from the number of wheels and from the base specifications for that model number. Right now base specifications are part of the class itself and are hardcoded into the class, along with the computation formulas.

My Issue: in the (legacy) code I am working with, I cannot always use the model number directly. It may not be available. Instead, an unrelated product_id is given, where the link between product_id and model_number is in the database.

I want to keep my class as simple as possible. As such I do not want to introduce another constructor that allows product_id as an input parameter, in addition to the existing model_number, as it will be duplication of code. Also, I am not too keen on putting database logic inside the class, but maybe I can, as thinking about it now, this may be a good candidate for a database-wrapper class, i.e. active record (where my base specifications can be moved off from the code and into the database).

Question: How do I create and return the object while following good object oriented principles when the model_number I typically use for its creation is not available, but another parameter is available instead (product_id in this case), which links 1:1 to model_number?

Sample solutions that I don't quite like:

  • Do not involve the database -- since I am working with just a few Products at this time, and Products do not change often, I can create a "conversion function" that serves as a map between product_id and model_number and not touch the database. Similarly, create a Convert object that has same functionality and use it before creating the object. Problem: duplicating DB functionality in the code.
  • Put DB functionality inside code and add alternate constructor. Problem: multiple entry-points of Product creation for the class create code duplication.

Update:

There basically are 3 aspects here:

  • base data (whether hardcoded or part of the database)
  • finding base data by various parameters be it model number or other identifying information
  • doing computation using base data as per model number

5 Answers 5

1

I'd like to build upon the solution @TMN is presenting and take it a little further to answer your SRP problem. Putting database code inside the ProductFactory would be non-sensical so to encapsulate this responsibility you can create a separate class ProductModelNumberProvider with just one method findByProductId(int productId).

The ProductFactory should provide a setter setProductModelNumberProvider(ProductModelNumberProvider lookupProvider) which will assign the provider internally and call findByProductId() when appropriate.

You will have a ProductFactory which is responsible for building the product and a ProductModelNumberProvider which will do the database interaction if needed.

Have a look at your StackOverflow question for a more concrete example.

4
  • special thanks to Bart for helping me here and on SO as I negotiated various OO concepts with what I already knew
    – Dennis
    Mar 26, 2014 at 20:21
  • I am accepting this answer as it introduces concept of Provider and overall gave me a fuller picture of what it means to do OO with SRP.
    – Dennis
    Mar 27, 2014 at 14:44
  • I am curious: it is stated that Provider is not a pattern (blog.ploeh.dk/2011/04/27/Providerisnotapattern) ad reading more about it, it seems that generally people dislike using this construct. I was curious to ask you if there is a need to reconsider using this construct?
    – Dennis
    Mar 31, 2014 at 21:54
  • Provider is just a part of the name. Don't think in patterns too much :) If I called it a Helper would it make any difference?
    – Bart
    Mar 31, 2014 at 22:37
7

Maybe create a ProductFactory, with methods to create a Product given a number of wheels and either a product ID or a model number. Given a model number it defers to the class' constructor, and if given a product ID it performs the DB lookup, gets the model number, then defers to the class constructor.

2
  • in this case, is it okay to put DB lookup and object creation in one class? I am a little lost trying to super-separate classes by using SRP, but maybe I'm taking it too far.
    – Dennis
    Mar 25, 2014 at 14:54
  • 2
    @Dennis The SRP can apply on different levels. For example, the purpose of the factory is to build a Product object given the input parameters. So the factory has all the logic it needs to do just that, including the necessary logic to get info from a DB. Still one single responsibility, just on a higher level. Of course your factory shouldn't have SQL queries in it, but it can call a function or two on a repository object.
    – Phil
    Mar 25, 2014 at 22:01
2

You don't want to duplicate the DB mapping product_id -> model_number by hardcoding the logic, that makes sense. So the first thing you need is a function like

int mapProductIdToModelNumber(int product_id)

which does this mapping by getting the information from the database (consider caching when this function is used very often).

What remains is how to make this function available to your Product class without tight coupling. You have the following alternatives:

  • pass the function directly to the code blocks whereever Product objects are created (depends on your programming language how to accomplish that)

  • implement the function in a ProductFactory class. Derive ProductFactory from an interface IProductFactory and pass an object of this type to the code areas where you currently want a new Product(). Use the factory the way @TMN suggested.

That way, your legacy code stays decoupled from the database, since it won't depend directly from a database-bound ProductFactory, only from IProductFactory, which can be easily mocked out (for example, for testing purposes).

9
  • Thanks.. it sounds similar to a database wrapper, where mapProductIdToModelNumber is same or similar to findModelById(), which I can also call from a ProductFactory. But then why pass it as a function, why not instantiate the Object where the database is being used and call that function from the object? (Except in my case it is legacy code and there is no database class, so I see your point on just passing the function using some language facility...)
    – Dennis
    Mar 24, 2014 at 21:00
  • @Dennis: the central point is the separation of concerns. Passing mapProductIdToModelNumber will allow you, for example, to exchange the function against one without any DB access (which might be helpful for testing purposes). It will also allow you not only to keep your Product class decoupled from the database, but also you ProductFactory.
    – Doc Brown
    Mar 24, 2014 at 21:11
  • 1
    While I agree on separation of concerns, adding additional indirection through lookup method passing is unnecessary complexity. If you already have such function, then there is no real need for factory itself and you can just call the function and then call the constructor. The single responsibility of the factory would be to encapsulate of the lookup so the calling code doesn't have to worry about how the product is created. Simply said "If you have to tell factory how to create the object, then there is no point in the factory."
    – Euphoric
    Mar 24, 2014 at 21:16
  • @Euphoric: the OP wants the computational code for creation in one place, and the database code in another, decoupled. So the natural solution I see is to put the computational code into the factory, and inject the db mapping function into the factory. Mixing these two things into one function or class will be a showstopper for unit testing.
    – Doc Brown
    Mar 24, 2014 at 21:22
  • 1
    @Dennis: my first suggestion is not procedural, but functional. My second suggestion is a more object oriented version of the first one.
    – Doc Brown
    Mar 25, 2014 at 22:01
1

Maybe this -- give up the idea of a Product class and create a DB-wrapper where I will say $pf = ProductFinder(), then do $pf->findById() and $pf->findByModel() as needed. Problem: Where do I put my computation code? It can't be in the same place, as it it will violate Single Responsibility Principle. aka, computation and data handling should be separate. Then maybe Product class can be the computational part, and ProductFinder be the database pulling part.

0

What I actually ended up doing:

At first I was wondering if I am better off with such complexity.

Simpler way was:

There is another pattern here that I could use that keeps things in one class: https://stackoverflow.com/a/2175213/2883328. Breaks SRP.

$product = Product::fromProductId($productId, $wheels);

SRP way (chosen)

  • ProductData class that just has the data (can be swapped by database later)
  • ProductFactory class that uses Provider class to look up the model number
  • ProductNumberProvider class that provides model number when given product id
  • Product class that does loads ProductData and does computations

Code:

$provider = new ProductModelNumberProvider();
$factory = new ProductFactory($provider);
$product = $factory->constructFromProductId($productId, $wheels);

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.