2

UPDATE 2017/04/19

Another view is using wrappers for late binding.

Introduction

I believe that objects should be immutable, so I only set properties via the constructor. In that case, the object state never change.

Problem

There are cases the required parameters for the constructor aren't known at that moment. So I need:

  • a setter (not my preference)
  • a 'hard coded' instance in my method (no Depency Injection, not flexible) (not my preference)
  • a deferred binding (possible solution, but the disadvantage (I think) is that the container contains too many logic)

Case

Let me give a simplified code example to illustrate my problem (PHP).

interface Personable
{
    public function __toString();
}

final class Person implements Personable
{
    private $name;

    public function __construct(string $name)
    {
        $this->name = $name;
    }

    public function __toString(): string
    {
        return 'Person ' . $this->name;
    }
}

final class Foo
{
    private $personable;

    public function __construct(Personable $personable)
    {
        $this->personable = $personable;
    }
}

Question

Let's say that I default bind Personable to Person in my container. At that moment I haven't the required name parameter. I want to pass the instance of Person via constructor. What's the best practice?

schellingerht
  • 175
  • 1
  • 11
  • related (possibly a duplicate): [Dependency Injection: Field Injection vs Constructor Injection?](http://softwareengineering.stackexchange.com/questions/300706/dependency-injection-field-injection-vs-constructor-injection) – gnat Apr 06 '17 at 10:26

3 Answers3

4

I think that your Foo class should not depend on a Personable instance. Your Foo class probably has a number of methods that you can call to make it perform some actions using the Personable object, however I would argue that objects like Personable should be passed as an argument to those functions.

Class Foo
{
    public function printName(Personable $personable) 
    {
        echo (string)$personable;
    } 
} 

Try to inject stateless dependencies (services, repositories, factories, etc.) via the constructor and inject these stateful objects (database models and such) via the function that you are calling.

Thijs Riezebeek
  • 336
  • 1
  • 6
  • I get your point. Pass the instance, but not set? – schellingerht Apr 07 '17 at 07:49
  • 1
    Exactly, `Personable` is an actionable object. It does not provide functionality that the class `Foo` depends on, but it is the object that is being used by `Foo`, therefor it should be passed as an argument to the function instead of having `Foo` depend on it. If you let `Foo` depend on it, you won't be able to reuse a `Foo` instance because it will always act on the same `Personable` instance. – Thijs Riezebeek Apr 07 '17 at 14:37
1

First off:

There are cases the required parameters for the constructor aren't known at that moment.

I dont really believe that. And even if so, there might be a more fundamental flaw to your applications architecture.

But assuming that nothing can be done about that core issue:


Say you want to Inject a dependency like this:

interface MyDependecy {
    function doStuff(): void;
}

class MyDependencyImpl1 implements MyDependency {
    function doStuff(): void { /* stuff */ }
}

class MyDependecyImpl2 implements MyDependency {
    function doStuff(): void { /* stuff */ }
}

You cannot decide between MyDependencyImpl1 and MyDependencyImpl2 at the time when __construct of the dependent object executes.

This is how i'd resolve it:

class MyDependencyDelegator implements MyDependency {
    /** @var ?MyDependency */ private $target;

    function __construct(?MyDependency $target) { $this->setTarget($target); }

    function doStuff(): void { $target->doStuff(); }

    function setTarget(MyDependecy $target) { $this->target = $target; }
}

In the DI config:

$delegator = new MyDependencyDelegator();
$this->when(MyDependentObject::class)->needs(MyDependecy::class)->give($delegator); // i dont remember the exact syntax right now, forgive me

// call $delegator->setTarget when the desicion can be made

Now, this boils down to setter injection. It does not resolve the fact that information is not present when it should be (at construction time of the dependent object). This approach just keeps the "dirty" part out of your core object and moves it closer to the issue: the DI config.

marstato
  • 4,538
  • 2
  • 15
  • 30
  • Thank you for your answer. Firstly, it's not about decision between implementations. My issue is: I need a `name` to make an instance of `Person`. The instance of `Person` is made in IoC container, because `Person` is bind (default) to the interface `Personable`. – schellingerht Apr 06 '17 at 11:48
  • And where is that name obtained from? – marstato Apr 06 '17 at 11:49
  • You're fast ;-) For example from the `Request` object. – schellingerht Apr 06 '17 at 11:50
  • Having a break at work ;) Thats your issue: in laravel, the applications object tree should be independent from the incoming request (might be good for java, but highly debatable in PHP IMHO). And you want to use request-dependent information in a part of your application that shouldn't do so. I see a few ways to solve the particular issue, but none of them are *nice* or *best practice* :/ – marstato Apr 06 '17 at 11:53
  • You're absolutely right. I think it's simply solvable by using setters (rather not), or passing as `local parameter` (without setting as global). – schellingerht Apr 06 '17 at 11:59
  • You could create an implementation of `Personable` that uses the `Request` Facade to obtain the name - but i'd consider that ugly :/ – marstato Apr 06 '17 at 12:03
  • Let us [continue this discussion in chat](http://chat.stackexchange.com/rooms/56643/discussion-between-schellingerht-and-marstato). – schellingerht Apr 06 '17 at 12:04
1

Since argument for creating an instance of Personable is unknown during creation of Foo - you need create it later.

You can introduce PersonableFactory abstraction, which will be responsible for creation of Personable.

interface PersonableFactory
{
    public function create();
}

final class PersonFactory implements PersonableFactory
{
    public function create(string $name): string
    {
        return new Person($name);
    }
}

final class Foo
{
    private $personableFactory;

    public function __construct(PersonableFactory$personableFactory)
    {
        $this->personableFactory = $personableFactory;
    }
}

In case when Person is just a data object, I think you don't need factory or dependency injection - just create new instance when you name will be known

Fabio
  • 3,086
  • 1
  • 17
  • 25
  • Thank you! But without DI, I need to make an instance hard coded, which makes it hard to test and maintain. – schellingerht Apr 06 '17 at 14:18
  • @schellingerht what do you mean by "make an instance hardcoded"? DI is for wiring up the "immutable" parts of your application - the code "paths" that can be set up at the start of the application. Things that need to be created at request time... need to be created at request time. It sounds like you're overthinking things. – Ant P Apr 06 '17 at 14:33
  • @AntP, thank you. I mean this: `public function bar(string $name): void { $person = new Person($name); }` – schellingerht Apr 06 '17 at 14:50
  • @schellingerht, with factory you will be able to test and maintain. For test you can "mock" factory and return object with expected data. But if `Person` is only data object - have no behaviour - only data - then you can simply create it and test that data is expected – Fabio Apr 06 '17 at 14:50
  • @schellingerht I don't see anything wrong with that. "new" is not evil. That's not the idea behind DI. – Ant P Apr 06 '17 at 14:52
  • 1
    @AntP, `new` consider "bad" practice from testing point of view, because you will not be able to "mock" behaviour of the created object. But it is ok if created object have no behaviour - only data. – Fabio Apr 06 '17 at 14:53
  • Thank you both. I know, but what about flexibility? `new` means I cannot easy change the implementation. That's why I prefer to pass it via constructor :-) – schellingerht Apr 06 '17 at 14:54
  • @schellingerht - can you show full code of `Person` class? Or at least tell us does this class have some functionality or it only "data object"? – Fabio Apr 06 '17 at 14:55
  • 1
    @Fabio new definitely isn't considered "bad practice from a testing point of view" - that depends entirely on the boundaries of the component and tests in question. What is considered bad practice is explicitly instantiating *dependencies* inside their dependants because it makes defining boundaries for unit tests difficult, but there's no evidence that this is what's happening here. – Ant P Apr 06 '17 at 14:59
  • 1
    @AntP, by "bad practice" I meant exactly this: _explicitly instantiating dependencies inside their dependants_ :) – Fabio Apr 06 '17 at 15:00
  • Thank you :-) Normally, I don't use an object as data bag. – schellingerht Apr 07 '17 at 07:50
  • I edited my question, because I've new info. What about using a wrapper class? – schellingerht Apr 19 '17 at 13:23
  • Can you show example of wrapper class you want to use? – Fabio Apr 19 '17 at 13:27