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

[Michael M Slusarz]

Quoting Chuck Hagenbuch <chuck at horde.org>:

> Quoting Michael M Slusarz <slusarz at horde.org>:
>
>> Quoting Chuck Hagenbuch <chuck at horde.org>:
>>
>>> commit 85e306b9042c49634b25d0a3d1573366576fc165
>>> Author: Chuck Hagenbuch <chuck at horde.org>
>>> Date:   Thu Oct 21 15:23:46 2010 -0400
>>>
>>>   Don't throw a warning if the global doesn't exist
>>>
>>> framework/Core/lib/Horde/Core/Browser.php |    7 +++++--
>>> 1 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> http://git.horde.org/diff.php/framework/Core/lib/Horde/Core/Browser.php?rt=horde-git&r1=a7c7c74a95a0a3947850d2f9b4bc51365b69c17a&r2=85e306b9042c49634b25d0a3d1573366576fc165
>>
>> There's no need to check the indices of a session variable - if the  
>> index doesn't exist, Horde_Session will automatically return null  
>> instead of throwing an undefined index error.
>>
>> It must be $GLOBALS['session'] itself that doesn't exist so the  
>> check should be for that.  (Not a big deal, but figured it was a  
>> good place to identify an advantage of Horde_Session since I  
>> haven't finished writing the docs yet).
>
> 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.

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

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')

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).

Obviously, we could add get/set method calls but I would rather be  
consistent with usage.

> 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.

As mentioned above, if worried about confusion, then I have no issue  
with renaming the global variable to something like 'hsession'.

As far as a long-haul project - it's not really all that long haul.   
imp, horde (mostly - except for Horde_Data stuff), turba, core, and  
kronolith have already been converted.  I have nag and info mostly  
converted already locally.  There is a bunch of session access code in  
various framework libraries that needs to be moved into core, but  
these are many of the same libraries that need to be fixed for H4  
anyway.

michael

-- 
___________________________________
Michael Slusarz [slusarz at horde.org]