82

When designing a system I am often faced with the problem of having a bunch of modules (logging, database acces, etc) being used by the other modules. The question is, how do I go about providing these components to other components. Two answers seem possible dependency injection or using the factory pattern. However both seem wrong:

  • Factories make testing a pain and don't allow easy swapping of implementations. They also don't make dependencies apparent (e.g. you're examining a method, oblivious to the fact that it calls a method that calls a method that calls a method that uses a database).
  • Dependecy injection massively swells constructor argument lists and it smears some aspects all over your code. Typical situation is where constructors of more than half classes look like this (....., LoggingProvider l, DbSessionProvider db, ExceptionFactory d, UserSession sess, Descriptions d)

Here's a typical situation I have a problem with: I have exception classes, which use error descriptions loaded from the database, using a query which has parameter of user language setting, which is in user session object. So to create a new Exception I need a description, which requires a database session and the user session. So I'm doomed to dragging all these objects across all my methods just in case I might need to throw an exception.

How do I tackle such a problem??

  • 2
    If a factory can solve all your problems, maybe you could just inject the factory into your objects and get LoggingProvider, DbSessionProvider, ExceptionFactory, UserSession from that. – Giorgio Mar 11 '13 at 13:49
  • 1
    Too many "Inputs" to a method, be they passed or injected, is more a problem of the methods design itself. Whichever you go with you might want to reduce the size of your methods a bit (which is easier to do once you get injection in place) – Bill K Mar 11 '13 at 19:47
  • The solution here shouldn't be reducing the arguments. Instead build abstractions that build a higher level object that does all the work in the object and gives you benefit. – Alex Oct 13 '16 at 9:10
74

Use dependency injection, but whenever your constructor argument lists become too big, refactor it using a Facade Service. The idea is to group some of the constructor arguments together, introducing a new abstraction.

For example, you could introduce a new type SessionEnvironment encapsulating a DBSessionProvider, the UserSession and the loaded Descriptions. To know which abstractions make sense most, however, one has to know the details of your program.

A similar question was already asked here on SO.

  • 9
    +1: I think grouping constructor arguments into classes is a very good idea. It also forces you to organize these argument into more meaning structures. – Giorgio Mar 11 '13 at 13:51
  • 5
    But if the result is not a meaningful structures then you are just hidding complexity that violates SRP. In this case a class refactoring should be done. – danidacar Sep 15 '13 at 13:22
  • 1
    @Giorgio I disagree with a general statement that "grouping constructor arguments into classes is a very good idea.". If you qualify this with "in this scenario" then it's different. – tymtam Sep 16 '16 at 0:29
19

Dependecy injection massively swells constructor argument lists and it smears some aspects all over your code.

From that, it doesn't seem like you understand DI proper - the idea is to invert the object instantiation pattern inside of a factory.

Your specific problem seems to be a more general OOP problem. Why can't the objects just throw normal, non-human-readable exceptions during their runtime, and then have something before the final try/catch that catches that exception, and at that point uses the session information to throw a new, prettier exception?

Another approach would be to have an exception factory, which is passed to the objects through their constructors. Instead of throwing a new exception, the class can throw on a method of the factory (e.g. throw PrettyExceptionFactory.createException(data).

Keep in mind that your objects, apart from your factory objects, should never use the new operator. Exceptions are generally the one special case, but in your case they might be an exception!

  • 1
    I have read somewhere that when your parameter list gets too long it is not because you are using dependency injection but because you need more dependency injection. – Giorgio Mar 11 '13 at 13:52
  • That could be one of the reasons - generally, depending on the language, your constructor should have no more than 6-8 arguments, and no more than 3-4 of those should be objects themselves, unless the specific pattern (like the Builder pattern) dictates it. If you're passing parameters to your constructor because your object instantiates other objects, that's a clear case for IoC. – Jonathan Rich Mar 11 '13 at 13:55
12

You have already listed the disadvantages of the static factory pattern quite well, but I don't quite agree with the disadvantages of the dependency injection pattern:

That dependency injection requires you to write code for each dependency is a not a bug, but a feature: It forces you to think about whether you really need these dependencies, thereby promoting loose coupling. In your example:

Here's a typical situation I have a problem with: I have exception classes, which use error descriptions loaded from the database, using a query which has parameter of user language setting, which is in user session object. So to create a new Exception I need a description, which requires a database session and the user session. So I'm doomed to dragging all these objects across all my methods just in case I might need to throw an exception.

No, you're not doomed. Why is it the responsibility of the business logic to localize your error messages for a particular user session? What if, sometime in the future, you wanted to use that business service from a batch program (which doesn't have a user session ...)? Or what if the error message should not be shown to the currently logged in user, but his supervisor (who may prefer a different language)? Or what if you wanted to reuse business logic on the client (which doesn't have access to a database ...)?

Clearly, localizing messages depends on who looks at these messages, i.e. it is the responsibility of the presentation layer. Therefore, I'd throw ordinary exceptions from the business service, that happen to carry a message identifier that can then be looked up the presentation layer's exception handler in whatever message source it happens to use.

That way, you can remove 3 unnecessary dependencies (UserSession, ExceptionFactory, and probably descriptions), thereby making your code both simpler and more versatile.

Generally speaking, I'd only use static factories for things you need ubiquitous access to, and that are guaranteed to be available in all environments we could ever want to run the code (such as Logging). For everything else, I'd use plain old dependency injection.

  • This. I can't see the need to hit the DB to throw an exception. – Caleth Jul 31 '18 at 12:39
1

Use dependency injection. Using static factories is an employment of the Service Locator antipattern. See the seminal work from Martin Fowler here - http://martinfowler.com/articles/injection.html

If your constructor arguments become too large and you're not using a DI container by all means write your own factories for instantiation, allowing it to be configurable, either by XML or binding an implementation to an interface.

  • 5
    Service Locator is not an antipattern - Fowler himself references it in the URL you posted. While the Service Locator pattern can be abused (in the same way that Singletons are abused - to abstract away global state), it is a very useful pattern. – Jonathan Rich Mar 11 '13 at 13:37
  • 1
    Interesting to know. I've always heard it referred to as an anti pattern. – Sam Mar 11 '13 at 13:45
  • 4
    It's only an antipattern if the service locator is used to store global state. The service locator should be a stateless object after instantiation, and preferably immutable. – Jonathan Rich Mar 11 '13 at 13:57
  • XML is not type safe. I would consider it a plan z after every other approach has failed – Richard Tingle Oct 18 '15 at 16:41
1

I would go as well with Dependency Injection. Remember that DI is not only done through constructors, but also through Property setters. For example, the logger could be injected as a property.

Also, you may want to use an IoC container that may lift some of the burden for you, for example by keeping the constructor parameters to things that are needed at runtime by your domain logic (keeping the constructor in a way that reveals the intention of the class and the real domain dependencies) and maybe inject other helper classes through properties.

A step further you may want to go is Aspect-Oriented Programmnig, which is implemented in many major frameworks. This can allow you to intercept (or "advise" to use AspectJ terminology) the constructor of the class and inject the relevant properties, maybe given a special attribute.

  • 4
    I avoid DI through setters because it introduces a window of time during which the object is not in fully initialized state (between constructor and setter call). Or in other words it introduces a method call order (must call X before Y) which I avoid if at all possible. – RokL Jan 10 '14 at 13:05
  • 1
    DI through property setters is ideal for optional dependencies. Logging is a good example. If you need logging, then set the Logger property, if not, then don't set it. – Preston Feb 19 '14 at 17:58
1

Factories make testing a pain and don't allow easy swapping of implementations. They also don't make dependencies apparent (e.g. you're examining a method, oblivious to the fact that it calls a method that calls a method that calls a method that uses a database).

I don't quite agree. At least not in general.

Simple factory:

public IFoo GetIFoo()
{
    return new Foo();
}

Simple injection:

myDependencyInjector.Bind<IFoo>().To<Foo>();

Both snippets serve the same purpose, they set up a link between IFoo and Foo. Everything else is just syntax.

Changing Foo to ADifferentFoo takes exactly as much effort in either code sample.
I've heard people argue that dependency injection allows for different bindings to be used, but the same argument can be made about making different factories. Choosing the right binding is exactly as complex as choosing the right factory.

Factory methods do allow you to e.g. use Foo in some places and ADifferentFoo in other places. Some may call this good (useful if you need it), some may call this bad (you could do a half-assed job at replacing everything).
However, it's not all that hard to avoid this ambiguity if you stick to a single method which returns IFoo so that you always have a single source. If you don't want to shoot yourself in the foot, either don't hold a loaded gun, or make sure to not aim it at your foot.


Dependecy injection massively swells constructor argument lists and it smears some aspects all over your code.

This is why some people prefer to explicitly retrieve dependencies in the constructor, like this:

public MyService()
{
    _myFoo = DependencyFramework.Get<IFoo>();
}

I've heard arguments pro (no constructor bloat), I've heard arguments con (using the constructor enables more DI automation).

Personally, while I've yielded to our senior who wants to use constructor arguments, I've noticed an issue with the dropdownlist in VS (top right, to browse the current class' methods) where the method names go out of sight when one of the method signatures is longer than my screen (=> the bloated constructor).

On a technical level, I don't care either way. Either option takes about as much effort to type. And since you're using DI, you won't usually call a constructor manually anyway. But the Visual Studio UI bug does make me favor not bloating the constructor argument.


As a side note, dependency injection and factories are not mutually exclusive. I've had cases where instead of inserting a dependency, I've inserted a factory which generates dependencies (NInject luckily allows you to use Func<IFoo> so you don't need to create an actual factory class).

The use cases for this are rare but they do exist.

  • OP asks about static factories. stackoverflow.com/questions/929021/… – Basilevs Jul 31 '18 at 13:41
  • @Basilevs "The question is, how do I go about providing these components to other components." – Anthony Rutledge Jul 31 '18 at 14:10
  • 1
    @Basilevs Everything I said, other than the side note, applies to static factories as well. I'm not sure what you're trying to specifically point out. What's the link in reference to? – Flater Jul 31 '18 at 16:11
  • Could one such use case of injecting a factory be such a case where one has devised an abstract HTTP request class, and strategized five other polymorphic child classes, one for GET, POST, PUT, PATCH, and DELETE? You cannot know which HTTP method will be used each time when trying to integrate said class hierarchy with an MVC type router (which may rely in part on an HTTP request class. PSR-7 HTTP Message Interface is ugly. – Anthony Rutledge Feb 21 at 4:27
  • @AnthonyRutledge: DI factories can mean two things (1) A class which exposes multiple methods. I think this is what you're talking about. However, this isn't really particular to factories. Whether it's a business logic class with several public methods, or a factory with several public methods, is a matter of semantics and makes no technical difference. (2) The DI-specific use case for factories is that a non-factory dependency is instantiated once (during injection), whereas a factory version can be used to instantiate the actual dependency at a later stage (and possibly multiple times). – Flater Feb 21 at 7:35
0

In this mock example, a factory class is used at runtime to determine which kind of inbound HTTP request object to instantiate, based on the HTTP request method. The factory itself is injected with an instance of a dependency injection container. This allows the factory to make its runtime determination and let the dependency injection container handle the dependencies. Each inbound HTTP request object has at least four dependencies (superglobals and other objects).

<?php
namespace TFWD\Factories;

/**
 * A class responsible for instantiating
 * InboundHttpRequest objects (PHP 7.x)
 * 
 * @author Anthony E. Rutledge
 * @version 2.0
 */
class InboundHttpRequestFactory 
{
    private const GET = 'GET';
    private const POST = 'POST';
    private const PUT = 'PUT';
    private const PATCH = 'PATCH';
    private const DELETE = 'DELETE';

    private static $di;
    private static $method;

    // public function __construct(Injector $di, Validator $httpRequestValidator)
    // {
    //    $this->di = $di;
    //    $this->method = $httpRequestValidator->getMethod();
    // }

    public static function setInjector(Injector $di)
    {
        self::$di = $di;
    }    

    public static setMethod(string $method)
    {
        self::$method = $method;
    }

    public static function getRequest()
    {
        if (self::$method == self::GET) {
            return self::$di->get('InboundGetHttpRequest');
        } elseif ((self::$method == self::POST) && empty($_FILES)) {
            return self::$di->get('InboundPostHttpRequest');
        } elseif (self::$method == self::POST) {
            return self::$di->get('InboundFilePostHttpRequest');
        } elseif (self::$method == self::PUT) {
            return self::$di->get('InboundPutHttpRequest');
        } elseif (self::$method == self::PATCH) {
            return self::$di->get('InboundPatchHttpRequest');
        } elseif (self::$method == self::DELETE) {
            return self::$di->get('InboundDeleteHttpRequest');
        } else {
            throw new \RuntimeException("Unexpected HTTP request. Invalid request.");
        }
    }
}

The client code for an MVC type setup, within a centralized index.php, might look like the following (validations omitted).

InboundHttpRequestFactory::setInjector($di);
InboundHttpRequestFactory::setMethod($httpRequestValidator->getMethod());
$di->set('InboundHttpRequest', InboundHttpRequestFactory::getRequest());
$router = $di->get('Router');  // The Router class depends on InboundHttpRequest objects.
$router->dispatch(); 

Alternatively, you can remove the static nature (and keyword) of the factory and allow a dependency injector to manage the whole thing (hence, the commented out constructor). However, you will have to change some (not the constants) of the class member references (self) to instance members ($this).

  • Downvotes without comments are not useful. – Anthony Rutledge Jul 31 '18 at 18:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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