[dev] [commits] Horde branch master updated. 0729ad4e199098207fec994fbac9d34c72e9b436

[Michael Rubinsky]

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

> Quoting Chuck Hagenbuch <chuck at horde.org>:
>
>> commit 0729ad4e199098207fec994fbac9d34c72e9b436
>> Author: Chuck Hagenbuch <chuck at horde.org>
>> Date:   Sun Oct 3 10:41:27 2010 -0400
>>
>>    Revert "Add ondemand injector binders." These are equivalent to  
>> factories.
>>
>>    This reverts commit e27690055fdcf4896b1a11cdecec1de304c1cffc.
>>
>> framework/Injector/lib/Horde/Injector.php               |   55  
>> +--------------
>> framework/Injector/package.xml                          |    2 +-
>> framework/Injector/test/Horde/Injector/InjectorTest.php |    8 --
>> 3 files changed, 4 insertions(+), 61 deletions(-)
>>
>> http://git.horde.org/diff.php/framework/Injector/lib/Horde/Injector.php?rt=horde-git&r1=e27690055fdcf4896b1a11cdecec1de304c1cffc&r2=0729ad4e199098207fec994fbac9d34c72e9b436
>> http://git.horde.org/diff.php/framework/Injector/package.xml?rt=horde-git&r1=e27690055fdcf4896b1a11cdecec1de304c1cffc&r2=0729ad4e199098207fec994fbac9d34c72e9b436
>> http://git.horde.org/diff.php/framework/Injector/test/Horde/Injector/InjectorTest.php?rt=horde-git&r1=e27690055fdcf4896b1a11cdecec1de304c1cffc&r2=0729ad4e199098207fec994fbac9d34c72e9b436
>
> Please revert this -- ondemand injector binders are NOT equivalent  
> to factories.  Instead, they are a way to reduce overhead during  
> initialization of the injector.  There is a not-insignificant amount  
> of overhead in reading all the binder class files from disk (even if  
> using a cache), creating the PHP data class objects, and storing  
> these class objects in memory.  Considering that most of the binders  
> are not used on any given pageload, this is unncessary overhead.   
> Additionally, it makes reading debug logs much easier - instead of  
> having to scroll through several pages of binder print_r() class  
> string output, there is only a single line for each declared binder.
>
> Additionally, it can be argued that the current behavior of  
> addBinder() is wrong.  Say, for example, some configuration value  
> used in a binder changes during the page load.  In the current  
> system, the old value of the configuration will *always* be used.   
> Using ondemand binders, the new value of the configuration may be  
> used if the binder is used after the configuration value changes.
>
> I would be all for allowing addBinder() to accept either an object  
> or a string and then auto-determining how to handle.

Perhaps a summary of the discussions that led to this will help  
illuminate. The bigger picture here is that our current use of binders  
is wrong. The majority, if not all, of the classes that exist in  
horde/core/binders should actually be factories, and that is what we  
are currently working towards. In this way, the on-demand binder *is*  
essentially the same as a factory binder...it creates the object  
on-demand, not when registered.

In the case of complex factories that require configuration parameters  
to be passed to the create() method, we should be requesting a factory  
directly from the injector, and not an instance of the object we want  
e.g.,  
$injector->getInstance('Horde_Core_Factory_Foo')->create($someParameters)  
instead of incorrectly adding a new binder that returns a factory.

For objects, such as shares, where the application scope needs to be  
passed to the factory, we discussed implementing a new type of binder  
that works similarly to the way the autoloader works, in that we parse  
the type of object being requested and figure out the scope from the  
name of the requested object. So e.g., when requesting something like  
$injector->getInstance('Kronolith_Share') the injector would know to  
get a share object and scope it to Kronolith.



-- 
Mike

--
The Horde Project (www.horde.org)
mrubinsk at horde.org

Mike

?The only reason for time is so that everything doesn't happen at  
once." - A. Einstein