rdlowrey / auryn

IoC Dependency Injector
MIT License
723 stars 65 forks source link

Sharing an alias does not work as expected. #155

Closed schlessera closed 1 year ago

schlessera commented 7 years ago

When I define separate aliases that are both implemented by the same class, and share these aliases, they don't work as I would expect them to. Instead of having two separate instances that are reused, there's only one instance, with the configuration of the alias that happened to be first or last (I think it overrides settings).

As an example:

class Employee {};

$injector->alias( 'Director', 'Employee' );
$injector->alias( 'Janitor',  'Employee' );

$injector->share( 'Director' );
$injector->share( 'Employee' );

$director = $injector->make( 'Director' );
$janitor =  $injector->make( 'Janitor'  );

In this example, I would expect the $director and the $janitor to be two different instances, but they are actually the same instances. To make this work as I expect, I need to add actual Director and Janitor class/interface stubs.

kelunik commented 7 years ago

To make this work as I expect, I need to add actual Director and Janitor class/interface stubs.

What do you mean by that? Could you show a modified example that works as expected?

schlessera commented 7 years ago

Yes, this would work as expected:

class Employee {};
class Director extends Employee {};
class Janitor  extends Employee {};

$injector->share( 'Director' );
$injector->share( 'Employee' );

$director = $injector->make( 'Director' );
$janitor =  $injector->make( 'Janitor'  );

$director and $janitor are now two separate instances of different classes that share the same base class.

In the non-working example above, I would expect the sharing to be done at the alias level, as it is done here at the subclass level. But the aliases do count for sharing.

morrisonlevi commented 7 years ago

I understand this may just be an example, but I highly doubt that you'd want to share any employee classes at all. This makes it more difficult to understand why you'd want aliases to be shared independently.

guilhermeaiolfi commented 7 years ago

Just to help @schlessera , another example could be Adapters for accessing DBs. You want to use the same class 'Adapter' to create the connection and share them with your application. But you still need to access other databases within the same application (using the same Adapter class). Expected behavior:


class DbAdapter {}

$injector->alias('WriteDB', 'DbAdapter');
$injector->alias('ReadDb', 'DbAdapter');

$injector->share('WriteDb');
$injector->share('ReadDb');

$writer = $injector->make('WriteDb');
$reader = $injector->make('ReadDb');

$writerB = $injector->make('WriteDb'); 
$readerB = $injector->make('WriteDb'); 

var_dump($writer === $writerB); // bool(true)
var_dump($writer === $reader); // bool(false)
var_dump($reader === $readerB); // bool(true)
var_dump($writerB === $readerB); // bool(false)

All four instances are DbAdapter instances, but since it was its aliases that were shared, Only the shared aliases would be shared not its class.

Danack commented 7 years ago

This:

$injector->alias('WriteDB', 'DbAdapter');
$injector->alias('ReadDb', 'DbAdapter');

confuses me. Aliasing it meant to be from more general type to a more specific type, but you appear to be using it the other way round?

Regardless, I generally recommend avoiding using aliasing magic to accomplish architecture. Instead using context objects and delegate functions to create them

class BackupDataContext {
     private $readDB;
     private $writeDB;

     function __construct(DbAdapter $readDB, DbAdapter $writeDB, ) {
         $this->readDB = $readDB;
         $this->writeDB = $writeDB;
     }
     // getters go here.
}

function createBackupDataContext(DbAdapterFactory $dbFactory) {
    return new BackupDataContext(
         $dbFactory->create('readDB'),
         $dbFactory->create('writeDB'),
   );
}

Using context objects makes it much easier to reason about what is happening in your application compared to using alias magic, when you have more than one implementation of some types.

guilhermeaiolfi commented 7 years ago

I'm end up creating an DbManager object that handles the magic for me. I think it's better this way, actually.

Em 23 de ago de 2017 16:25, "Danack" notifications@github.com escreveu:

This:

$injector->alias('WriteDB', 'DbAdapter'); $injector->alias('ReadDb', 'DbAdapter');

confuses me. Aliasing it meant to be from more general type to a more specific type, but you appear to be using it the other way round?

Regardless, I generally recommend avoiding using aliasing magic to accomplish architecture. Instead using context objects and delegate functions to create them

class BackupDataContext { private $readDB; private $writeDB;

 function __construct(DbAdapter $readDB, DbAdapter $writeDB, ) {
     $this->readDB = $readDB;
     $this->writeDB = $writeDB;
 }
 // getters go here.

}

function createBackupDataContext(DbAdapterFactory $dbFactory) { return new BackupDataContext( $dbFactory->create('readDB'), $dbFactory->create('writeDB'), ); }

Using context objects makes it much easier to reason about what is happening in your application compared to using alias magic, when you have more than one implementation of some types.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rdlowrey/auryn/issues/155#issuecomment-324437579, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPhmKdDT5tQMPQPqsN0eE_2ywza1nIHks5sbHyggaJpZM4Mqefs .

Danack commented 1 year ago

I added some more words about advanced patterns in the readme which probably suggests solutions to this type of problem.