Config keys and dependency injection

[Paul M Jones-4]

Hi all,

Jeff brought to my attention a while ago that it can be difficult to  
tell at-a-glance which config keys are intended for dependency  
injection (more technically "service location" but it's close  
enough).  Since DI/SL is a core part of how Solar works, I think it  
might be good to begin some sort of convention for these config keys.

For example, right now if you need a Solar_Sql injection, the key is  
typically 'sql' and the value is also 'sql' (indicating "use the  
registry object named 'sql' for this injection"), like so:

     protected $_Class_Name = array(
         'sql' => 'sql',
     );

Would it be useful to do something like one of the following, as  
visual indicator (by convention) that the value for that key is for a  
dependency injection?

     protected $_Class_Name = array(

         '@sql' => 'sql',

         '*sql' => 'sql',

         '_sql' => 'sql',

         '+sql' => 'sql',

         ':sql' => 'sql',

     );

This might be dumb or not-useful.

If not dumb and indeed useful, then which of those prefixes seems most  
reasonable? What alternative prefixes might be good?  Note that a lot  
of single-characters prefixes might look strange or imply something  
else, like $ for a variable, & for a reference, <> for comparisons, etc.

Comments?


--

Paul M. Jones
http://paul-m-jones.com/

_______________________________________________
Solar-talk mailing list
Solar-talk@...
http://mailman-mail5.webfaction.com/listinfo/solar-talk

[Dmytro Konstantinov]

Hi everyone,

Paul and Jeff have raised a very interesting point... I can't say that  
it has been that much of an issue for me but I do see what Jeff means:  
sometimes it really is difficult to tell at-a-glance which config keys  
are intended for dependency injection. Plus, I think that making  
things clearer will do no harm, so I'm all for it.

However, I do believe that whatever prefix we choose (to identify such  
a key), should be intuitive, easy to guess, and ideally should fit in  
nicely with already established conventions.. Looking at the list of  
the prefixes Paul suggests I would say that given current conventions  
within Solar, PHP and my own personal preferences I would interpret  
them as following:

_ (eg _sql) - personally I interpret this as more of a visibility  
operator, similarly to the current scheme used to indicate public vs  
protected variables.

+ (eg +sql) - to me, this is either an addition operator or an  
indicator of the read/write access to the following key (eg +sql is  
read-write, -sql is read-only).

* (eg *sql) - once again, to me, an asterisk is instantly associated  
with a wild card or a quantifier for a regular expression.

: (eg :sql) - personally I see colon as one of those universal symbols  
that are good for allot of things. I my case, it is also strongly  
associated with namespacing in Perl (aka packages).

@ (eg @sql) - following an already established convention, "at"  
indicates a beginning of a reserved keyword used by the  
phpDocumentator or Solar's own documentation system.

In other words, all of the above symbols could be interpreted in  
multiple ways and this is by no means an "objective review", just my  
opinion. But based on that opinion and considering that we are talking  
about identifying a key that will be injected with a registry object,  
I would opt out for something that would:
       
        either reminded me of an object interface, such as ":" (eg :sql)
        or reminded me of a reserved keyword, such as "@" (eg @sql)

Hope this helps,

Dmytro


On 15 Oct 2009, at 21:46, Paul M Jones wrote:

> Hi all,
>
> Jeff brought to my attention a while ago that it can be difficult to
> tell at-a-glance which config keys are intended for dependency
> injection (more technically "service location" but it's close
> enough).  Since DI/SL is a core part of how Solar works, I think it
> might be good to begin some sort of convention for these config keys.
>
> For example, right now if you need a Solar_Sql injection, the key is
> typically 'sql' and the value is also 'sql' (indicating "use the
> registry object named 'sql' for this injection"), like so:
>
>     protected $_Class_Name = array(
>         'sql' => 'sql',
>     );
>
> Would it be useful to do something like one of the following, as
> visual indicator (by convention) that the value for that key is for a
> dependency injection?
>
>     protected $_Class_Name = array(
>
>         '@sql' => 'sql',
>
>         '*sql' => 'sql',
>
>         '_sql' => 'sql',
>
>         '+sql' => 'sql',
>
>         ':sql' => 'sql',
>
>     );
>
> This might be dumb or not-useful.
>
> If not dumb and indeed useful, then which of those prefixes seems most
> reasonable? What alternative prefixes might be good?  Note that a lot
> of single-characters prefixes might look strange or imply something
> else, like $ for a variable, & for a reference, <> for comparisons,  
> etc.
>
> Comments?
>
>
> --
>
> Paul M. Jones
> http://paul-m-jones.com/
>
> _______________________________________________
> Solar-talk mailing list
> Solar-talk@...
> http://mailman-mail5.webfaction.com/listinfo/solar-talk

_______________________________________________
Solar-talk mailing list
Solar-talk@...
http://mailman-mail5.webfaction.com/listinfo/solar-talk

[Paul M Jones-4]

On Oct 15, 2009, at 16:55 , Dmytro Konstantinov wrote:

> In other words, all of the above symbols could be interpreted in
> multiple ways and this is by no means an "objective review", just my
> opinion. But based on that opinion and considering that we are talking
> about identifying a key that will be injected with a registry object,
> I would opt out for something that would:
>
> either reminded me of an object interface, such as ":" (eg :sql)
> or reminded me of a reserved keyword, such as "@" (eg @sql)
>
> Hope this helps,

Dymtro, it does help, thanks. I have the same associations/intuitions  
you list ("_" for private/protected, +/* as math, etc) and I'm glad to  
find that someone else sees it that way too.

Maybe a "^sql" since it looks like a control character, and DI/SL is  
also known as "inversion of control" (IOC) ?

Or maybe it's a non-issue, and we should address other stuff.




--

Paul M. Jones
http://paul-m-jones.com/




_______________________________________________
Solar-talk mailing list
Solar-talk@...
http://mailman-mail5.webfaction.com/listinfo/solar-talk

[Dmytro Konstantinov]

On 16 Oct 2009, at 02:22, Paul M Jones wrote:

> Maybe a "^sql" since it looks like a control character, and DI/SL is
> also known as "inversion of control" (IOC) ?

Yep, ^ (eg ^sql) works for me too and I don't have any existing  
associations with "^" character. So it could work very well indeed!

Though visually, I think I prefer ":". I'd say it is a little bit less  
intrusive and easier to read in the code but I can't give you any  
better reason then that. I just like the way it looks! :) On the other  
hand, "^" seems to be allot more meaningful .... I guess it's just a  
matter of taste, either one would be absolutely fine by me.


> Or maybe it's a non-issue, and we should address other stuff.

Personally, this issue never bothered me all that much but now that I  
think about it, I agree with Jeff: if the DI/SL keys were easily  
identifiable at-a-glance then it would improve code readability and  
maintenance.

I would say that if it is something that can be done in a day then it  
is well worth doing it, however if it breaks too many things and  
requires a major rewrite of the code then it is probably best left as  
it is.

Dmytro
_______________________________________________
Solar-talk mailing list
Solar-talk@...
http://mailman-mail5.webfaction.com/listinfo/solar-talk

[Paul M Jones-4]

On Oct 16, 2009, at 09:56 , Dmytro Konstantinov wrote:

>
> On 16 Oct 2009, at 02:22, Paul M Jones wrote:
>
>> Maybe a "^sql" since it looks like a control character, and DI/SL is
>> also known as "inversion of control" (IOC) ?
>
> Yep, ^ (eg ^sql) works for me too and I don't have any existing
> associations with "^" character. So it could work very well indeed!
>
> Though visually, I think I prefer ":". I'd say it is a little bit less
> intrusive and easier to read in the code but I can't give you any
> better reason then that. I just like the way it looks! :) On the other
> hand, "^" seems to be allot more meaningful .... I guess it's just a
> matter of taste, either one would be absolutely fine by me.

I prefer the ":" too.

>> Or maybe it's a non-issue, and we should address other stuff.
>
> Personally, this issue never bothered me all that much but now that I
> think about it, I agree with Jeff: if the DI/SL keys were easily
> identifiable at-a-glance then it would improve code readability and
> maintenance.
>
> I would say that if it is something that can be done in a day then it
> is well worth doing it, however if it breaks too many things and
> requires a major rewrite of the code then it is probably best left as
> it is.

For the core Solar stuff, it won't break anything (it's a find-and-
replace).  COnfig files, though, will need some manual attention,  
otherwise you'll key on 'sql' when you should be keying on ':sql'.

Anyone else have thoughts here?



--

Paul M. Jones
http://paul-m-jones.com/




_______________________________________________
Solar-talk mailing list
Solar-talk@...
http://mailman-mail5.webfaction.com/listinfo/solar-talk

[Robert Gonzalez-2]

Might sound a bit picky, but the colon character usage (":sql") could potentially be confused with PDO Statement placeholder keys (not to the framework, but to the eye). Just saying, that is the first thing I thought of when I saw it in your example.

On Fri, Oct 16, 2009 at 8:22 AM, Paul M Jones <pmjones@...> wrote:

On Oct 16, 2009, at 09:56 , Dmytro Konstantinov wrote:

>
> On 16 Oct 2009, at 02:22, Paul M Jones wrote:
>
>> Maybe a "^sql" since it looks like a control character, and DI/SL is
>> also known as "inversion of control" (IOC) ?
>
> Yep, ^ (eg ^sql) works for me too and I don't have any existing
> associations with "^" character. So it could work very well indeed!
>
> Though visually, I think I prefer ":". I'd say it is a little bit less
> intrusive and easier to read in the code but I can't give you any
> better reason then that. I just like the way it looks! :) On the other
> hand, "^" seems to be allot more meaningful .... I guess it's just a
> matter of taste, either one would be absolutely fine by me.

I prefer the ":" too.

>> Or maybe it's a non-issue, and we should address other stuff.
>
> Personally, this issue never bothered me all that much but now that I
> think about it, I agree with Jeff: if the DI/SL keys were easily
> identifiable at-a-glance then it would improve code readability and
> maintenance.
>
> I would say that if it is something that can be done in a day then it
> is well worth doing it, however if it breaks too many things and
> requires a major rewrite of the code then it is probably best left as
> it is.

For the core Solar stuff, it won't break anything (it's a find-and-
replace).  COnfig files, though, will need some manual attention,
otherwise you'll key on 'sql' when you should be keying on ':sql'.

Anyone else have thoughts here?

--

Paul M. Jones
http://paul-m-jones.com/




_______________________________________________

Solar-talk mailing list
Solar-talk@...
http://mailman-mail5.webfaction.com/listinfo/solar-talk

--

Robert Gonzalez
http://www.robert-gonzalez.com

_______________________________________________
Solar-talk mailing list
Solar-talk@...
http://mailman-mail5.webfaction.com/listinfo/solar-talk

[Jeff Moore-4]

On Oct 15, 2009, at 1:46 PM, Paul M Jones wrote:

> For example, right now if you need a Solar_Sql injection, the key is
> typically 'sql' and the value is also 'sql' (indicating "use the
> registry object named 'sql' for this injection"), like so:
>
>     protected $_Class_Name = array(
>         'sql' => 'sql',
>     );
>
> Would it be useful to do something like one of the following, as
> visual indicator (by convention) that the value for that key is for a
> dependency injection?
>
>     protected $_Class_Name = array(
>
>         '@sql' => 'sql',
>
>         '*sql' => 'sql',
>
>         '_sql' => 'sql',
>
>         '+sql' => 'sql',
>
>         ':sql' => 'sql',
>
>     );

I think its actually the right hand side that requires clarification,  
not the left hand side.

         'sql' => '@sql',

         'sql' => '^sql',

         'sql' => '_sql',

         'sql' => '+sql',

         'sql' => ':sql',

What if you start out as a plain key then migrate to "SI" (Solar  
Injection).  Rename city.  That seems like an implementation detail  
leaking into the interface to me.

These types of references shouldn't change depending on how you're  
instantiating the object:

$this->_config['sql'];
Solar::Factory('Foo', array('sql' => $sql));


That said, lets brainstorm a bit ...


We could just abandon the string syntax entirely and use an array.

Defining a config option:

        'sql' => array(Solar::REGISTRY => 'sql')

I'd consider this fairly understandable.


Another thing to consider might be changing this to allow a class and  
config combination instead of just config array.

        'sql' => array(Solar::FACTORY => array( 'Solar_Sql',  
array('profiling' => true)))

I'd consider this to be more understandable, if more verbose than the  
current

        'sql' => array('profiling' => true)

it has the advantage that you can specify the class in config.

At that point, maybe its not necessary at all to have  
Solar::dependency().


Now lets go into highly theoretical land ...

Consider what would happen if Solar::factory was responsible for  
injecting configuration into an object instead of Solar_Base pulling  
it in.  (I did this when I was working on optimizing config.  Its  
technically feasible and no slower that what's there now.)

So __construct would just set the $config:

        $this->_config = $config;


Then, we can eliminate solar::dependency().  Solar::factory can take  
this over by iterating through its $config parameter before  
instantiating the objects

foreach ($config as $key => $value) {
        if (is_array($value)) {
                if (array_key_exists(Solar::FACTORY, $value)) {
                     $dependency_class = $value[Solar::FACTORY][0];
                     $dependency_config = $value[Solar::FACTORY][1];
                     $config[$key] = Solar::factory($dependency_class,  
$dependency_config);
                }
                if (array_key_exists(Solar::REGISTRY, $value)) {
                     $config[$key] =  
Solar_Registry::get($value[Solar::REGISTRY]);
                }
        }
}

This moves all the "Solar injection / Service Locator" logic into a  
standard modern style of dependency injection container.

You can inject any registry value into any class for any key without  
changing that class.

It would work great with registry lazy loading.

You can use the more flexible factory notation to instantiate  
different classes.  You can nest these.  You can do it for any config  
option of the class without changing the class.

You can eliminate the pre and post config because config would be done  
before the object is instantiated.

You could eliminate alot of Solar_Registry direct access in favor of  
config options.  This would make things easier to write test cases for.

Solar_Base gets smaller.

It can grow to use standard objects.  For example, injecting directly  
to a property instead of a config option, using a setter, or a  
constructor signature other than ($config).

The drawbacks are the constant is a bit magic and there would be a  
small performance hit to scan the config options.  I can see scenarios  
where it would be faster.


Thoughts?

Jeff
_______________________________________________
Solar-talk mailing list
Solar-talk@...
http://mailman-mail5.webfaction.com/listinfo/solar-talk

[Paul M Jones-4]

Jeff,

Good stuff, man.  I'm going to skip past the near-term stuff and get  
to the end, which sounds best -- but I have a question.

> Now lets go into highly theoretical land ...
>
> Consider what would happen if Solar::factory was responsible for
> injecting configuration into an object instead of Solar_Base pulling
> it in.  (I did this when I was working on optimizing config.  Its
> technically feasible and no slower that what's there now.)
>
> So __construct would just set the $config:
>
> $this->_config = $config;

I have to say, I like the sound of this.

Here's my question: right now, the class default configs are in the  
$_Vendor_Class_Name property, which means you have to be inside the  
class to work through the parent configs, so that configs get  
inherited properly.  How would the hypothetical Solar::factory()  
"injection" deal with that part of it?  I think you're alluding to it  
when you say this ...


> The drawbacks are the constant is a bit magic and there would be a
> small performance hit to scan the config options.

... i.e., "scan the config options."

Can you expand on your remarks there?





--

Paul M. Jones
http://paul-m-jones.com/




_______________________________________________
Solar-talk mailing list
Solar-talk@...
http://mailman-mail5.webfaction.com/listinfo/solar-talk

[Jeff Moore-2]

On Oct 21, 2009, at 6:43 PM, Paul M Jones wrote:

>> So __construct would just set the $config:
>>
>> $this->_config = $config;
>
> I have to say, I like the sound of this.

Cool.

> Here's my question: right now, the class default configs are in the
> $_Vendor_Class_Name property, which means you have to be inside the
> class to work through the parent configs, so that configs get
> inherited properly.  How would the hypothetical Solar::factory()
> "injection" deal with that part of it?  I think you're alluding to it
> when you say this ...

Right.  You can get this with reflection.  Which is what I did when  
experimenting with config performance before.  it was about the same  
performance as the current solution.

>> The drawbacks are the constant is a bit magic and there would be a
>> small performance hit to scan the config options.
>
> ... i.e., "scan the config options."
>
> Can you expand on your remarks there?

Compiling the config options is the same speed.  However,  
Solar::factory would have to iterate over them and look for the  
special constants indicating where it should replace values with  
values from the registry or where it should recursively call itself to  
create objects.

Jeff


_______________________________________________
Solar-talk mailing list
Solar-talk@...
http://mailman-mail5.webfaction.com/listinfo/solar-talk