[dev] [commits] Horde branch master updated. 85e306b9042c49634b25d0a3d1573366576fc165

[Chuck Hagenbuch]

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

>> Is there a reason that Horde_Session uses a global with array  
>> semantics? Makes it feel very magic, and not being familiar with  
>> its internals, I had no way of knowing the above - nor does anyone  
>> looking at how it's used. In fact I didn't even know that  
>> $GLOBALS['session'] was a Horde_Session object.
>
> That's simply because the documentation hasn't been finalized yet.

Not entirely. I might not have known where to start in the docs just  
from the global.

> I thought of using hsession instead of session, but went for the  
> slightly shorter name because I thought it is clearer.

Why not horde_session? I don't mind a little extra length for full clarity.

> Session data is essentially a key -> lookup action so array  
> semantics work great.  I am definitely NOT for the idea of having  
> some method do this for us.  For me, I would much rather  
> type/use/read:
>
> $session['horde:foo']
>
> than
>
> $session->getValue('horde', 'foo')

There are a few other options that still give direct clues that you're  
dealing with an object:

$horde_session->get('horde', 'foo');
$horde_session->get('horde:foo');
$horde_session->horde->foo;

I'd prefer any of those. I agree that the full getValue() version is  
unnecessary.

> I find it MUCH easier to read the former rather than the latter  
> (it's one of the reasons why I would support changing  
> $prefs->setValue() and $prefs->getValue() calls to using array  
> semantics - the method calls are just way too wordy for something  
> that is used a bunch).

I think we already have support for __get/__set there, so you can use  
$prefs->value. I do like that pattern for objects like this.

>> I know we need to keep the global for a while at least, and  
>> injecting it everywhere is going to be a long-haul project, but is  
>> there a reason not to at least use method-call semantics on it to  
>> make it clearer what you're interacting with?
>
> Confused what you mean by "keep the global for a while"?   
> Horde_Session will always be global.  It is no different than  
> $registry or $prefs - basic components of the framework that should  
> be easily available everywhere.

$registry is potentially a tricky one, but I don't think $prefs should  
always stay global, and $session (or $horde_session) probably not  
either. They should be injected where they are needed. Now we should  
definitely have helpers available that make them available easily  
within templates and controllers with no extra work. But even in Core  
packages these should be passed in explicitly when they're needed.

-chuck