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

[Gunnar Wrobel]

Quoting Chuck Hagenbuch <chuck at horde.org>:

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

A very strong +1 from my side.

I know that dependency injection may sometimes look like overhead or  
seem cumbersome. Or it can make you ask yourself if this is still PHP  
you are doing rather than Java.

But I have seen what the benefits of explicitly declaring your  
dependencies are. I have hands-on experience on how a group of coders  
can drive the Horde3 applications into a pure code desaster. And that  
was in part due to the use of global state in Horde3. Which is at  
least one factor that can make it extremely hard for outsiders to  
understand how some internals work.

And then I know the successor of that code desaster. And one reason  
why the same group of coders can make this successor fly is because  
the code is much easier to grasp. Each class explicitly declares what  
it is going to work on. And you may guess which dependency injection  
framework is being used for this successor ;)

Of course there are limits (see the recent discussion Jan and I had on  
Horde_Translation) and you can still produce unreadable code even if  
injecting dependencies. But I feel the benefits outweigh the potential  
problems by far and I'm be happy if we use injection where ever  
possible.

Cheers,

Gunnar

>
> -chuck
>
> -- 
> Horde developers mailing list - Join the hunt: http://horde.org/bounties/
> Frequently Asked Questions: http://horde.org/faq/
> To unsubscribe, mail: dev-unsubscribe at lists.horde.org
>