Contextual Bindings / Strategy Pattern Proof of Concept

[defunctl]

Hello!

There have a been a few requests for this functionality in the past:

1. How can factory supply many instances when using DI Container with constructor injection #807
2. One interface to multiple classes #702
3. Strategy pattern #609
4. A proxy should call Container::get() instead of creating a new instance itself #291

I took a stab at coding this functionality using a custom Definition Helper based on the AutoWireDefinitionHelper here: https://github.com/moderntribe/tribe-libs/pull/110/files

I'd be curious if you'd be open to getting similar functionality directly in PHP-DI, instead?

Other containers do support some form of this functionality:

1. Laravel
2. Symfony
3. etc...

Please see the preliminary tests for how these definitions could be used to...

return [
	// Bind multiple concrete classes to the same interface:
	RedColorManager::class   => Container\autowire()->contextualParameter( ColorInterface::class, Red::class ),
	BlueColorManager::class  => Container\autowire()->contextualParameter( ColorInterface::class, Blue::class ),
	GreenColorManager::class => Container\autowire()->contextualParameter( ColorInterface::class, Green::class ),

	// Or, with a callback:
	ColorManager::class => Container\autowire()->contextualParameter( ColorInterface::class, static fn () => new Red() ),

	// Or, a callback with container injection
	ColorManager::class => Container\autowire()->contextualParameter( ColorInterface::class, static fn ( ContainerInterface $c ) => $c->get( Red::class ) ),

	// Or, an external factory
	ColorManager::class => Container\autowire()->contextualParameter( ColorInterface::class, static fn ( ContainerInterface $c ) => $c->get( ColorFactory::class )->makeUserSelectedColor() ),

	// Or, a basic inline factory: $color_state = 'blue';
	ColorManager::class => Container\autowire()->contextualParameter( ColorInterface::class, static function ( ContainerInterface $c ) use ( $color_state ) {
		switch ( $color_state ) {
			case 'red':
				return $c->get( Red::class );
			case 'blue':
				return $c->get( Blue::class );
			case 'green':
				return $c->get( Green::class );
			default:
				throw new InvalidArgumentException( 'Invalid color' );
		}
	} ),
];

[SvenRtbg]

I think I have questions that your code (at least the tests around it) does not immediately answer:

You have three classes implementing the Color interface, and you have one ColorManager that is wrapping these Color classes somehow, and in order to create multiple instances of said ColorManager you have to use inheritance to multiply the definitions possible, one per color. What exactly is the benefit here? One could easily also modify the constructor, allowing explicitly only the red, blue or green class respectively, with classic autowiring (losing interchangeability, I see...). Or write explicit factories or constructorParameter definitions for these classes. You'd still have to write explicit definitions for these three ColorManagers anyway.

Your tests do not answer how many instances are being created in the process. Most of the time they are testing for the color string, alternatively they are testing instanceOf. Both ways cannot detect if the same instance is expected to be returned, or another instance. What is your approach supposed to do?

It appears to me that anything that isn't a builtin type now has to be defined using contextualParameter instead of continuing to allow constructorParameter. Otherwise your test could easily inject the SampleTask the old way. Am I mistaken here? At least there is a distinction between builtins and non-builtins, and I would not expect contextual parameters to fail for builtins. I am allowed to pass a callable, this can return a string for a string-parameter contextually, can't it?

The overall question: Does this allow doing something that is impossible without it, and how much hassle is the old method compared to the new one? I currently believe I would be able to change any of your example definitions above into some form of constructorParameter and adjust the parameters to that call a bit, and it would work.

[defunctl]

Hi @SvenRtbg,

I appreciate your reply. I'll try to respond to everything I can here.

First, here are the classes being used in the tests.

One could easily also modify the constructor, allowing explicitly only the red, blue or green class respectively, with classic autowiring (losing interchangeability, I see...

Correct, we would then be coupling concrete dependencies to classes, with a pattern like the strategy pattern and essentially the design by contract philosophy, this is the opposite of what we're trying to achieve by being able to switch out a dependency without modifying our classes and adding "new" work instead of editing existing work.

We would never have to touch the target class, but instead modify the factory to provide a different or new implementation. I coupled the tests by name for clarity, but you can imagine that RedColorManager::class must change to accept Blue::class due to whichever business requirements that changed.

Or write explicit factories or constructorParameter definitions for these classes. You'd still have to write explicit definitions for these three ColorManagers anyway.

The constructorParameter() is very fragile, since it relies on the name of the variable in the constructor and not the interface/abstract. This tightly couples the variable name to the container, and it's not uncommon to change the variable name of some dependency for whatever reason. A developer could easily break this implementation, where with contextualParameter() it uses the interface/abstract, so they can change the variable name all they want.

The power of programming to an interface here, is that I can implement a whole brand new crazy way to implement that interface, and that often means the constructor might need an additional dependency or two, which I believe could be be easier (see below).

You'd still have to write explicit definitions for these three ColorManagers anyway.

Yes, but I lose autowiring as far as I understand. I want to allow a developer to add a whole bunch of concrete injections without worrying it will break.

Your tests do not answer how many instances are being created in the process. Most of the time they are testing for the color string, alternatively they are testing instanceOf.

In these tests, we're essentially creating one per request. The color string is just a string that matches the class from which it comes from, e.g. green. I agree, I could just return the instance instead and test that for consistency, but it's essentially the same thing.

What is your approach supposed to do?

Allows me contextually bind a concrete to an interface, while keep autowiring for the object, and allowing the constructor order/variable names to be whatever they want to be without fear of breaking the implementation. In addition, allowing me to adjust the strategy of which concrete instance gets injected by only modifying the "service provider" or definer.

This also allows providing a different instance for a concrete class, tied to the specific parent that would be accepting the dependency.

It appears to me that anything that isn't a builtin type now has to be defined using contextualParameter instead of continuing to allow constructorParameter. Otherwise your test could easily inject the SampleTask the old way. Am I mistaken here? At least

That's currently correct, but I guess it doesn't have to be that way. What do you mean by "old way", via constructorParameter() using the constructor variable name?

and I would not expect contextual parameters to fail for builtins. I am allowed to pass a callable, this can return a string for a string-parameter contextually, can't it?

They currently don't fail for builtins, they would throw the same exceptions PHP-DI does now if you pass a string, bool, array without telling PHP-DI what those definitions are. I use the constructorParameter() here in conjunction with everything else as part of a single definition. But, I should write the reverse test to expect the exception.

The overall question: Does this allow doing something that is impossible without it, and how much hassle is the old method compared to the new one? I currently believe I would be able to change any of your example definitions above into some form of constructorParameter and adjust the parameters to that call a bit, and it would work.

If you'd be able to show me how to write an inline factory like this while fully maintaining autowiring and not returning an actual return new Red( ...whatever other random dependencies this could have and could change at any time ); while mapping that to a specific calling parent class like in that test, maybe I've been missing something this whole time?

The biggest benefit here is maintaining autowiring, so a class that implements an interface can have its constructor parameters change quite easily (if I wanted to add another concrete, where PHP-DI would just know to inject that).

I'm sure an example would help best here of what I'm trying to avoid/improve.

Let's say we have this class:

class ColorManager {

	private ColorInterface $color;
	
	public function __construct( ColorInterface $color ) {
		$this->color = $color;
	}
	
}

Right now, we can use a factory like this in a definer:

ColorManager::class => static fn ( ContainerInterface $c ) => new ColorManager( $c->get( Red::class  ) ),

Or we can globally bind a single concrete to a single interface, and we wouldn't need the above factory at all, but now we can't switch the strategy at runtime:

ColorInterface::class => static fn ( ContainerInterface $c ) => $c->get( Red::class ),

For the sake of argument, let's say we need add a few more concrete dependencies to the ColorManager constructor, all that can be autowired:

class ColorManager {

	private ColorInterface $color;
	private ColorModifier $modifier;
	private ColorSharpener $sharpener;

	public function __construct( ColorInterface $color, ColorModifier $modifier, ColorSharpener $sharpener ) {
		$this->color     = $color;
		$this->modifier  = $modifier;
		$this->sharpener = $sharpener;
	}
	
}

We have to now modify our factory in our definer:

ColorManager::class => static fn ( ContainerInterface $c ) => new ColorManager( $c->get( Red::class  ), $c->get( ColorModifier::class ), $c->get( ColorSharpener::class ) );

With the change I'm suggesting, it would lessen the amount of work, because the following definition would work for all the above situations, before and after the class changes, and without "globally" binding the interface to a single concrete class:

ColorManager::class => Container\autowire()->contextualParameter( Color::class, Red::class ),

Of course, the real power is the strategy configuration at runtime, where some other object determines the strategy, for example:

1. A user's stored settings, e.g. you have an interface for a payment processor, and multiple strategies that implement that strategy, and the user has "credit card" selected as their preferred method. The "Processor" object doesn't care which concrete an object is, as long as it implements the correct interface, and with this, they would get their selected option. As a developer I can also write a new payment processor and wire it up without touching a single one of my existing classes, just a new definition for PHP-DI and done.
2. Or, say something like User Roles, where each role implements a Role interface, a developer could provide the correct concrete class based on their role in a database, keeping all the objects stateless, and retrieving their specific role's instance.

As mentioned before, this brings similar functionality like Laravel's contextual bindings. In their documentation, PhotoController::class could have another 5 concrete dependencies in its constructor, in any order and we just need to tell it which concrete to give for the interface and that's it, the container does the rest, same as here.

I hope this helps and let me know if I can provide anything else.

[mnapoli]

Could you detail why this doesn't solve your problem:

return [

    ColorManager::class => autowire()->constructorParameter('color', factory(function () {
        return new Red();
    })),

];

[SvenRtbg]

I do agree with most of your statements, specifically that the constructorParameter is a bit cumbersome to use, and brittle when doing refactoring without paying attention.

Still I think your main argument is based on "runtime based configuration", with giving examples that all rely for the DI container definition to fetch some information that already requires the DI container to exist, then feed that information back into the container to proceed in the code path, influencing what really gets created. I believe that this is explicitly not a design goal anymore for PHP-DI.

It is designed to deliver statically definable object trees of stateless objects - that's what all that ability to compile the container is about: We'd have to have a final configuration at some point, after that the compilation process has all the information to put into faster build instructions. While it still allows to influence it's configuration or objects after the container is established, that isn't really a feature that implementors should rely on.

So when looking at the Laravel example:

$this->app->when(PhotoController::class)
      ->needs(Filesystem::class)
      ->give(function () {
          return Storage::disk('local');
      });

$this->app->when([VideoController::class, UploadController::class])
      ->needs(Filesystem::class)
      ->give(function () {
          return Storage::disk('s3');
      });

this is in fact a nice way to write a static definition, but you'd still be able to do it with PHP-DI right now, albeit having to write more "custom" closures.

Implementing any example like this would aim to improve the syntax of PHP-DI required for such a use case.

However, anything that uses "a users stored settings" or "user roles" is out of scope by definition. Surely getting hold of these settings would require to use objects, be it database access, reading cookies, querying the authentication subsystem or doing whatever is necessary. So the DI container is already established in order to get the info, which should not be looped back into the container to influence future object construction - and one of the reasons is that you'd have to very properly build the container to avoid using objects that are supposed to change based on the info put back into the container.

Yes, having another container just as the second-step container sounds like the next best plan, but in fact I'd rather go with strategy factories instead. They can easily be injected statically, and decide which implementation to provide based on however the information to make the decision is passed to them.

If you'd be able to show me how to write an inline factory like this

The closure doesn't do anything fancy. I'd assume that you would simply be able to just call constructorParameter with the parameters name instead of contextualParameter with the parameters type, and it would work right away. I don't like that this closure is pulling info from outside the container. If at runtime the info is available, I would expect the configuration to either dynamically decide which definition to add into the container instead of evaluating it inside the closure, or pull some info from the container itself to decide. The result is the same, but in both cases the implementor is forced to establish the known fact that is contained in $color_state before using the container instead of populating the variable somewhat later. Allowing this would allow to at least think it would be possible to set a value, fetch from the container, then change it and fetch again and get a different result. So if at all possible, avoid using these situations as an argument why your proposal is desirable - at least in my humble opinion. Currently it looks like some very artificial use case to me.

I'm thinking about the one statement that you made about constructorParameter being fragile. I do agree, it is. Why is an implementor using it? The most obvious case would be to pass undecidable types, scalars most of the time, into the class. Configuration strings, probably arrays. For any interfaces, you'd be able to either define one global lookup, or make an explicit statement as discussed. That might just be as easy as

ColorManager::class => Container\autowire()->constructorParameter( 'color', Red::class )

(seeing that @mnapoli commented while I was writing my comment, I'd thing that you don't even need the closure)

In general, influencing the way autowiring works currently requires stating code elements like parameter variable names or method names as strings. That's not desirable very much because of tight coupling with the code, affecting the ability to refactor at will. The same is not true for class names as long as they are written as ::class constants - any decent IDE will rename them. Still, the oldfashioned string style would work, but implementors have options to avoid the situation here. I'd like to see some improvement for this, however PHP does not really offer any engine support expressing what would be required here. There are essentially two existing alternative ways: constructorParameter requires you to specify the parameter name, and constructor uses positional parameters, i.e. you have to specify params starting from the first (possibly omitting the later ones to be resolved by autowiring), and the order of these parameters must not change during refactoring then.

[defunctl]

Could you detail why this doesn't solve your problem:

return [

    ColorManager::class => autowire()->constructorParameter('color', factory(function () {
        return new Red();
    })),

];

1. It makes refactoring fragile and more complicated. In a large application you may end up with a lot of definitions, and a developer could rename a variable in their class (not the type) and break everything. Tightly coupling the variable names to the definition just feels unnatural when that class accepts an object and the built in ::class constant exists.
2. I want to avoid directly instantiating any classes ( e.g. new Red();) and utilize autowiring as much as possible to maintain flexibility in class pattern design. If you had 10 concrete color classes, so say 10 of your example definitions above, if I want to auto inject a concrete instance in these, I now have to go back and change every definition, even though it would be supported by autowiring, basically not making use of a great feature.

[defunctl]

@SvenRtbg keep i mind with Laravel example you can do all of the logic I've detailed here, including utilizing the already defined or autowired definitions, looping back into the container, e.g.

$this->app->when([VideoController::class, UploadController::class])
      ->needs(Filesystem::class)
      ->give(function () {
          return $this->app->make( Storage::class )->disk('s3');
      });

// or
$this->app->when([VideoController::class, UploadController::class])
      ->needs(Filesystem::class)
      ->give( static function ( Application $app ) {
          return $app->make( Storage::class )->disk( 'local' );
      });

// or an inline factory/strategy like example
$this->app->when([VideoController::class, UploadController::class])
      ->needs(Filesystem::class)
      ->give( static function ( Application $app ) {
          $environment = $app->environment();

          switch ( $environment ) {
          		case 'production':
      			case 'staging':	
          			$storageStrategy = 's3';
      			default:
      				$storageStrategy = 'local';
          }
          
          return $app->make( Storage::class )->disk( $storageStrategy );
      });

[mnapoli]

Could you detail why this doesn't solve your problem:

return [

    ColorManager::class => autowire()->constructorParameter('color', factory(function () {
        return new Red();
    })),

];

1. It makes refactoring fragile and more complicated. In a large application you may end up with a lot of definitions, and a developer could rename a variable in their class (not the type) and break everything. Tightly coupling the variable names to the definition just feels unnatural when that class accepts an object and the built in `::class` constant exists.

So the problem is that you define the injection based on a variable name?

Since PHP now supports named arguments, argument names are now part of the API of a method. Furthermore, variables can be renamed and parameter types can be changed in the same way, I don't see one as more "robust" than the other.

2. I want to avoid directly instantiating any classes ( e.g. `new Red();`) and utilize autowiring as much as possible to maintain flexibility in class pattern design. If you had 10 concrete color classes, so say 10 of your example definitions above, if I want to auto inject a concrete instance in these, I now have to go back and change every definition, even though it would be supported by autowiring, basically not making use of a great feature.

The new Red was from your examples. You can do without that:

return [

    ColorManager::class => autowire()->constructorParameter('color', get(Red::class)),

];

Does that answer this point?

---

On a side note, TBH the discussion is extremely long and complex to follow. I'd love one concrete example from your project that fits in a few lines (if you have that).

[alexkuc]

@defunctl Out of curiosity, why not this?

return [
  ClassA::class => autowire()->constructor(get(Red::class)),
  ClassB::class => autowire()->constructor(get(Green::class)),
];

where Red and Green:

class Green implements ColorInterface

and consuming class is

public function __construct(ColorInterface $c)

In example of Laravel and Symfony, both are not "true" strategy as they still define concrete implementation in the config of service locator.

If we are in situation where we don't know what concrete argument value until we actually run the code, I think a standalone factory class would be a better choice as this way your code is more separated plus you can easily unit test it.