Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should factories set model properties?

As part of an overall S.O.L.I.D. programming effort I created a factory interface & an abstract factory within a base framework API.

People are already starting to overload the factories Create method. The problem is people are overloading the Create method with model properties (and thereby expecting the factory to populate them).

In my opinion, property setting should not be done by the factory. Am I wrong?

public interface IFactory
{
    I Create<C, I>();
    I Create<C, I>(long id); //<--- I feel doing this is incorrect

    IFactoryTransformer Transformer { get; }
    IFactoryDataAccessor DataAccessor { get; }
    IFactoryValidator Validator { get; }
}

UPDATE - For those unfamiliar with SOLID principles, here are a few of them:

Single Responsibility Principle
It states that every object should have a single responsibility, and that responsibility should be entirely encapsulated by the class

Open/Closed Principle
The meaning of this principle is that when a get a request for a feature that needs to be added to your application, you should be able to handle it without modifying old classes, only by adding subclasses and new implementations.

Dependency Inversion Principle
It says that you should decouple your software modules. To achieve that you’d need to isolate dependencies.

Overall:
I'm 90% sure I know the answer. However, I would like some good discussion from people already using SOLID. Thank you for your valuable opinions.

UPDATE - So what do I think a a SOLID factory should do?

IMHO a SOLID factory serves-up appropriate object-instances...but does so in a manner that hides the complexity of object-instantiation. For example, if you have an Employee model...you would ask the factory to get you the appropriate one. The DataAccessorFactory would give you the correct data-access object, the ValidatorFactory would give you the correct validation object etc.

For example:

var employee = Factory.Create<ExxonMobilEmployee, IEmployee>();
var dataAccessorLdap = Factory.DataAccessor.Create<LDAP, IEmployee>();
var dataAccessorSqlServer = Factory.DataAccessor.Create<SqlServer, IEmployee>();
var validator = Factory.Validator.Create<ExxonMobilEmployee, IEmployee>();

Taking the example further we would...

var audit = new Framework.Audit(); // Or have the factory hand it to you
var result = new Framework.Result(); // Or have the factory hand it to you

// Save your AuditInfo
audit.username = 'prisonerzero';

// Get from LDAP (example only)
employee.Id = 10;
result = dataAccessorLdap.Get(employee, audit);
employee = result.Instance; // All operations use the same Result object

// Update model    
employee.FirstName = 'Scooby'
employee.LastName = 'Doo'

// Validate
result = validator.Validate(employee);

// Save to SQL
if(result.HasErrors)
     dataAccessorSqlServer.Add(employee, audit);

UPDATE - So why am I adamant about this separation?

I feel segregating responsibilities makes for smaller objects, smaller Unit Tests and it enhances reliability & maintenance. I recognize it does so at the cost of creating more objects...but that is what the SOLID Factory protects me from...it hides the complexity of gathering and instantiating said objects.

like image 267
Prisoner ZERO Avatar asked Jan 04 '13 14:01

Prisoner ZERO


2 Answers

I'd say it's sticking to DRY principle, and as long as it's simple values wiring I don't see it being problem/violation. Instead of having

var model = this.factory.Create();
model.Id = 10;
model.Name = "X20";

scattered all around your code base, it's almost always better to have it in one place. Future contract changes, refactorings or new requirements will be much easier to handle.

It's worth noting that if such object creation and then immediately properties setting is common, then that's a pattern your team has evolved and developers adding overloads is only a response to this fact (notably, a good one). Introducing an API to simplify this process is what should be done.

And again, if it narrows down to simple assignments (like in your example) I wouldn't hesitate to keep the overloads, especially if it's something you notice often. When things get more complicated, it would be a sign of new patterns being discovered and perhaps then you should resort to other standard solutions (like the builder pattern for example).

like image 188
k.m Avatar answered Sep 30 '22 03:09

k.m


Assuming that your factory interface is used from application code (as opposed to infrastructural Composition Root), it actually represents a Service Locator, which can be considered an anti-pattern with respect to Dependency Injection. See also Service Locator: roles vs. mechanics.

Note that code like the following:

var employee = Factory.Create<ExxonMobilEmployee, IEmployee>();

is just syntax sugar. It doesn't remove dependency on concrete ExxonMobilEmployee implementation.

You might also be interested in Weakly-typed versus Strongly-typed Message Channels and Message Dispatching without Service Location (those illustrate how such interfaces violate the SRP) and other publications by Mark Seemann.

like image 20
Roman Boiko Avatar answered Sep 30 '22 02:09

Roman Boiko