[dev] [commits] Horde branch refactor-Notification updated. 3b590095a4312f85639071510adc42d5dc9090fc

[Gunnar Wrobel]

Quoting Jan Schneider <jan at horde.org>:

> Zitat von Gunnar Wrobel <p at rdus.de>:
>
>> Hi,
>>
>> as I need to use the Horde_Notification package in stand-alone mode  
>> I  was forced to refactor it a bit. The main problem was that the  
>> module  is firmly embedded into the global Horde context and  
>> contains a number  of calls to services such as Horde_Registry,  
>> Horde_Auth, Horde_Nls,  Horde::logMessage(). These service are of  
>> course missing if the  package is used on its own.
>>
>> I pushed the necessary refactorings into the  
>> "refactor-Notification"  branch. It would be nice if I could get  
>> some feedback on the changes.  Do they work or are there conflicts?  
>> Does the structure look okay? Can  I merge the branch?
>>
>> The main changes were two new components: The   
>> Horde_Notification_Storage and the Horde_Notification_Handler   
>> interfaces. The storage part allows to exchange the underlying  
>> storage  mechanism (for Horde this is the session by default). And  
>> the handler  part moves the references to global scope that were  
>> present in  Horde_Notification into separate decorators. This way  
>> they can be  avoided when using the Horde_Notification for itself.
>
> Looks good so far. Beside the small fixes that I already committed,  
> alarm snoozing doesn't work anymore though. It looks like the alarms  
> are being cached in the session somehow, but I didn't have to track  
> this down yet. That should be fixed before the merge.

Okay, will look into that.

>
> I'm not perfectly sure yet about the file and class name layout of  
> the decorators. The base handler and the decorators all implement  
> Horde_Notification_Handler, so it makes sense to have them in  
> Notification/Handler/. OTOH it might be more intuitive to make it  
> clearer that those are decorators, by using different class names.  
> Chuck might want to chime in here too.

I think that would be useful. I used decorators at a few places now  
and sometimes I had to check to determine if it was a "real" class or  
a decorator. I assume this would mean another subfolder in  
"Notification/Handler"? As the class names would start to get really  
long by then I'd tend to use "Notification/Handler/Deco" rather than  
"Notification/Handler/Decorator".

> Also, I don't recall right now if we already discussed whether we  
> want interfaces to stick more out by using a naming scheme for  
> those. I would prefer that, to distinguish them from base classes.
> All this is general Horde CS stuff of course, and has nothing to do  
> with the Notification refactoring. Meaning that we can always change  
> this later if don't find an immediate agreement.

"Notification/Handler/Interface.php"? Or  
"Notification/Interfaces/Handler.php"? Yes, something like that would  
be good.

Cheers,

Gunnar

>
> Jan.
>
> -- 
> Do you need professional PHP or Horde consulting?
> http://horde.org/consulting/
>
>
> -- 
> 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
>




-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digitale PGP-Unterschrift
URL: <http://lists.horde.org/archives/dev/attachments/20091030/087d54e0/attachment.bin>