[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,

I understood why you removed the "array" type hint for the parameters.  
Why is it required to use Horde_String though?

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

Yes, the message stacks were not cleared anymore. I implemented  
ArrayAccess for the storage handler and that was aparently a bad idea  
as these functions do not pass the values by reference. I will remove  
that tomorrow morning and merge the branch then.

Thanks for the feedback!

Gunnar


>
> 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.
> 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.
>
> 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/20091105/3e673a0d/attachment.bin>