[dev] Injector musings

[Chuck Hagenbuch]

Quoting Michael M Slusarz <slusarz at horde.org>:

> 1.) Is the eventual goal to remove all singleton methods from  
> libraries?  To me, the answer would seem to be yes because if we  
> eventually use getInstance()/createInstance() to reliably grab an  
> instance, if that library in the future would need an outside  
> dependency it would only be necessary to resolve this dependency by  
> creating a binder interface in Core.

Yes. I see no reason not to remove singletons and use the injector to  
manage number of instances.

> 2.) How do you create an instance with an injector if that  
> instance's constructor has varied arguments?  This seems to be a  
> limitation of Horde_Injector... is this by design or simply because  
> nobody has implemented it yet.

The latter, if I'm following. What I want to add to the injector (that  
came from a completely separate conversation with one of the folks who  
wrote the initial code) is setter injection. In the case of the  
Mime_Viewer objects, different viewers would have setFoo() methods for  
the dependencies that aren't in the constructor (common to all  
Mime_Viewers). The injector would then satisfy those dependencies as  
well on object creation.

> For example, Horde configuration needs to be decoupled from  
> Horde_Mime_Viewer.  However, every Horde_Mime_Viewer instance does  
> not need every single configuration option passed to it that might  
> potentially be used by the drivers.  On the other hand, the drivers  
> may create other Horde_Mime_Viewers internally that may need config  
> options not needed by the original driver.

For this last case the viewer should have a reference to the injector.  
Using setter injection this could be accomplished with a  
setInjector(Horde_Injector $injector) method; the injector would know  
to pass itself to this when creating the object.

> Regardless, it is very useful to keep the instance around since it  
> might be used elsewhere in the code.  A better example of this might  
> be IMP_Contents.  Once initialized, it may be used in various  
> locations around the code that are not accessible to each other.   
> How does this work with injector usage?  Or is this the motivation  
> to retain usage of singletons in this usage pattern?

I'm not sure I entirely follow the question. If the object  
instantiation/initialization is complicated, then a binder or factory  
for it seems appropriate. You can then call getInstance() wherever you  
need it, and the object will be created and initialized only once.

> 3.) Configuration buried deep within core library functions is  
> troublesome.  Example - Horde_Auth::setAuth(), which contains a call  
> to Net_DNS_Resolver, which may potentially be configured via Horde's  
> config file.  setAuth() is called from a variety of locations, and  
> is also called internally by a variety of other library calls.   
> Passing the resolver object as a parameter isn't really doable and  
> would be a giant step back in usability.  And it isn't possible to  
> pass the resolver object to Horde_Auth since it is not setup to be  
> OO for this kind of use.  Even if we convert to an OO-model,  
> Horde_Auth is called from so many places that if we have to add  
> additional parameters to 10's or 100's of calls, this seems like a  
> step back.

It seems like the static usage of Horde_Auth is a barrier here. If  
Horde_Auth were used non-statically then you could get a reference to  
it from the injector and then do what you needed without worrying  
about objects. We can also make things like the current username  
(getAuth) available as properties where appropriate so that they don't  
require accessing an object and a method call.

As an aside, I think one of the reasons there are potentially 100's of  
static Horde_Auth calls to worry about here is because of the kind of  
boilerplate code you've been arguing for removing/consolidating. In  
short: I agree. :)

At that point it doesn't seem unreasonable to pass the  
Net_DNS_Resolver object to Horde_Auth through either setter or  
constructor injection.

> My solution was to set a static public member variable to a  
> Net_DNS_Resolver object via the Horde_Registry.  It seems a bit  
> hackish, but also seems like the cleanest/easiest way to decouple  
> the configuration from the library.  Comments/concerns on this  
> approach is much appreciated.

Why not have a binder or factory to create the Net_DNS_Resolver  
object? I know this answer might not matter if Horde_Auth is still  
used statically, but see above.

-chuck