I'm trying to refactor some code, so it uses dependency injection.

Take this (non-sense) example class:

class Foo {
  protected $min;
  protected $max;
  public $bar;

  public function __construct($min, $max) {
    $this->min = $min;
    $this->max = $max;

    $this->bar = new Bar($this->calc());
  }

  public function calc() {
    $number = rand($this->min, $this->max);
    return new Quz($number);
  }
}

To use dependency injection (with containers), I would change this to:

class Foo {
  protected $barFactory;
  protected $quzFactory;

  protected $min;
  protected $max;
  public $bar;

  public function __construct(BarFactoryInterface $barFactory, QuzFactoryInterface $quzFactory) {
    $this->barFactory = $barFactory;
    $this->quxFactory = $quxFactory;
  }

  public function init($min, $max) {
    $this->min = $min;
    $this->max = $max;

    $this->bar = $this->barFactory->create($this->calc());
  }

  public function calc() {
    $number = rand($this->min, $this->max);
    return $this->quxFactory->create($number);
  }
}
  • Do I really need a factory method or class for each class?
  • Do I indeed need the init() function to replace the constructor?
  • Or am I doing it terribly wrong?
  • 1
    To answer the title: nope, factories aren't required. Ok.. now I read your question. – Theraot Oct 1 '16 at 3:17
  • @Theraot how can I implement the above class with DI but without factories? – Jasny - Arnold Daniels Oct 1 '16 at 3:20
up vote 5 down vote accepted

Do I really need a factory method or class for each class?

Since you are creating instances of "Bar" and "Quz" it makes sense to use factories to decouple them.

Yet, you are only creating a Bar in the constructor, so you could not take a factory for it, just take an instance of Bar instead. Doing so would be an example of dependency injection without a factory; it allows the client code provide the object… using a factory if that is the intention, or not. In fact, you are not using Bar, you can skip that.

Edit: I notice that you want to use the result of calc to create the Bar. Taking the factory instead of the Bar instance would give you a better encapsulation as it ensures that the Bar was created with a valid result for Quz given the min and max provided... yet, bar is public, and the client code could overwrite it which defies that protection.

As per Quz, since you will be creating a new instance each time the client code calls calc, you will need the factory to be able to get more instances.

Do I indeed need the init() function to replace the constructor?

You can take the factories in the constructor. Using a separate method creates a problem because it allows the developer to call the constructor and skip calling init.

Now, if you wanted to hide the factories and have the client code create multiple instances that share the same factories... well, you could create a factory for that. I have to say that there is no need to do that.

Or am I doing it terribly wrong?

Separating init from the constructor. Consider what happens if the client code creates an instance and calls calc without calling init: In your code the random number will either be 0 (or whatever default value) or trash because your field weren't initialized (depending on your language).

It could have been worst, if you actually used Bar, it wouldn't have been initialized because you do that in init, so it would be an invalid pointer.

Injecting objects or factories solves different purposes and communicate different intentions.

If you pass a factory, your new object may or may not create new objects from that factory. Those objects cannot be preconfigured (well, at least not without code smells like setting static properties on the factory). You can only inject things that have factories. Sometimes, this might be exactly what you want - but then, you can view at it just as injecting an ordinary object which happens to be a factory.

If you inject an "ordinary" object, you can pass anything - objects without factories are no problem. Creating factory classes just for that purpose seems ill-advised.

If you ask yourself why you want to code-inject, you probably want to pass configuration/state/behavior/whatever-you-call-it. Passing-down factories limits that very much. For example, how can you pass a database connector that couldn't be configured? And it couldn't be reused either.

I think you are mixing up concepts here (i.e. factory-pattern and dependency-injection) - which can be used together, but don't have to.

It generally makes sense to use factories when the two conditions are met:

  1. You don't know at compile time what your dependencies are
  2. The process of setting up those dependencies is "complicated"

When those two criteria are not met, just passing instances through the constructors works well enough and gives you the best bang for the buck.

The basic principle of the factory itself is criteria 2, so unless that is there, it doesn't even make sense to use a factory, DI or not.

The original Foo does three things:

  1. $this->bar = new Bar(new Quz(rand($min, $max))
  2. defines calc() as new Quz(rand($min, $max))
  3. remembers $min and $max so calc doesn't need them passed

The original Foo is already doing dependency injection. It's primarily construction. The rand thing is a touch of behavior and I really like to separate behavior from construction so having that hard coded in construction is a bit weird for me but not my chief gripe.

My chief gripe is I don't see any of this getting used. This reads like a contrived example. I really can't see that any of this is helping.

But fine. I'll imagine how the original Foo is used. Say I need a Bar made the way Foo makes a Bar. Oh, and I need some Quz's. So I do this:

$foo = new Foo(1,10);
$bar = $foo.bar;
$quz1 = $foo.calc();
$quz2 = $foo.calc();
$quz3 = $foo.calc();

All of that is construction code. All it bought me was not having to write it like this:

How can I implement the above class with DI but without factories?

$min = 1;
$max = 10;
$bar = new Bar(new Quz(rand($min, $max));
$quz1 = new Quz(rand($min, $max));
$quz2 = new Quz(rand($min, $max));
$quz3 = new Quz(rand($min, $max));

Which, I swear to god, is actually easier to read. It's also amazingly flexible. I'll inject what I please. I'll use whatever method I like on $min and $max.

But fine, let's see what we get when we switch to the improved Foo:

$foo = new Foo(new BarFactoryDefault(), new QuzFactoryDefault());
$foo.init(1,10);
$bar = $foo.bar;
$quz1 = $foo.calc();
$quz2 = $foo.calc();
$quz3 = $foo.calc();

Or am I doing it terribly wrong?

Yes

In addition to the above being more complicated than either previous solution I have yet to create these default factory classes, let alone the interfaces for them. Yes this is terribly wrong.

What your doing is reaching out to patterns and techniques you've heard were good and thought that by using good ingredients you were going to cook good food. Always taste the food you cook before you serve it. I like ketchup and I like milk. I don't like ketchup in my milk. That's not a problem with either the ketchup or the milk.

The problem here is the example is anemic. The problems these techniques solve aren't in this example. So trying to explore the techniques here becomes a festival of over engineering that doesn't teach you anything good.

You asked some other questions which I'll deal with next but the main point I want to make here is don't design these classes without thinking about how they're used.

Are factories required when doing dependency injection?

No.

Dependency injection doesn't require factories. Factories are one of many construction patterns. Dependency injection doesn't even require construction patterns. You can do it in main. Factories might perform injections. Factories can be injected. Factories abstract details of construction. You mostly use Factories to hide dependencies you want to treat as reasonable defaults. Least I do.

Do I really need a factory method or class for each class?

You need a factory method for each construction ceremony you are sick of looking at. It's crazy to think there is some 1 to 1 relationship here. It's zero to many.

Do I indeed need the init() function to replace the constructor?

I think it's an unpardonable sin that you let Foo exist in a non-initialized state. There are many ways to fix that. Fold it into the constructor. Break this up into two objects. Builder patterns. Just find some way to not force me into holding on to something I can't use if I don't follow it's ceremony.

Sorry this has turned into such a rant. I remember wading through design patterns and wondering what the fuss was about. Bad examples like this were a big reason it took me so long to get it. Please don't accept that this stuff is good until you can see why it's good. Faith is a killer here. Taste the food before you serve it.

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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