Sebastiaan's avatar

Looking for an alternative to instantiating bound classes or interfaces now that Laravel 5.4 removed passing parameters to app()->make()

So upgrading to Laravel 5.4 has unearthed a situation where I cannot apply a certain use case I apply from time to time. Envision the following example:

// Bind an interface to an implementation in a registered service provider
$this->app->bind(MyServiceInterface::class, function ($app, $parameters) {
    return new MyServiceA(...$parameters);
});

// Instantiate whatever class is bound to the interface with the given constructor arguments
$myService = app(MyServiceInterface::class, ['parameter1', 'parameter2', 'parameter3']);

Works perfectly in 5.3 and before, but in 5.4 Taylor removed —rather abruptly— constructor parameters from the make method invalidating this example. In effect, I cannot bind classes to interfaces (or IoC container strings to implementations) any longer if they require (primitive) parameters that cannot be auto-resolved. In my actual use case, I have for instance credentials I need to pass down to the service for which the service cannot be responsible to auto-retrieve from the config (as I would usually do).

The upgrade guide states that this is a code smell and thus he has enforced his own rule as such. Yet I remember this wasn't available in pre 5.x versions either, but was reintroduced/fixed in later versions due to rather high demand.

So my question is: is there an alternative for this? Basically I need interchangeable DI interfaces/classes, but with support for (primitive) constructor parameters as in 5.3 and before. Switching out MyServiceA for MyServiceB and so on which implement the same interface should be the goal.

Thanks!

0 likes
4 replies
Tabun's avatar

Just chiming in to say that this is the first thing that I ran into while reading the changelog for 5.4. I use this exact approach all the time.

In some cases, I can easily switch to making factories:

$something = app('some-factory')->make('object', ['param1', 'param2']);

In other cases, I'm not so sure that is a practicable approach.

I'm half considering writing a package that brings back the old App::make behaviour in a separate stateful 'container', just to make the transition easier.

Though long story short, I haven't decided either, yet, how to tackle this.

In any case, removing this IMHO pretty crucial bit of functionality with minimal explanation (just saying it has a code smell is rarely justification for making a pretty big breaking change) is a bit of a shock. I'm ready to learn what is so terrible about binding interfaces in the container this way, but it deserves a bit more than 3 short sentences in an upgrade guide... :]

2 likes
Tabun's avatar

As a follow up, for another application that has now gotten way more complicated that it would be with the old App::make behavior:

I'm testing a RestService class that instantiates a guzzle Client:

This used to be as follows:

$this->client = app(Client::class, [ $guzzleConfig ]);

Then, when testing it, I can easily mock the client to do a proper unit test:

$guzzleMock = $this->getMockBuilder(Client::class)->getMock();

// Guzzle testing setup here ...

app()->instance(Client::class, $guzzleMock);

This is no longer possible, so now I have to tightly couple the guzzle client like so:

$this->client = new Client($guzzleConfig);

This leaves me unable to unit test the RestService.

The solution to this that I'm using now is:

  • to create a GuzzleClientFactory class that performs the new Client() logic.
  • to create aGuzzleClientFactoryInterface interface that is bound to the GuzzleClientFactory
  • to let the RestService now instantiate the factory through the interface binding to make the client.

With this way more complicated setup I can again simply mock the factory. The only difference is that the factory is designed not to use constructor parameters.

This seems clunky, overwrought and arbitrary.

What am I missing here? What makes this less 'smelly' than what I had before?

2 likes
Sebastiaan's avatar

I agree and it's kind of a similar situation I had before using Guzzle.

Could you perhaps post your example in the Laravel internals issue I've created? See https://github.com/laravel/internals/issues/391 That way we can perhaps convince Taylor to bring it back, as it's just a commit that has to be reversed (or a few).

Tabun's avatar

Done, didn't know that issue thread existed. :]

Please or to participate in this conversation.