tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Hanik - Dev Lists <devli...@hanik.com>
Subject Re: svn commit: r536830 - in /tomcat/tc6.0.x/trunk/java/org/apache/catalina: CometEvent.java connector/CometEventImpl.java
Date Thu, 10 May 2007 12:28:23 GMT
Remy Maucherat wrote:
> fhanik@apache.org wrote:
>> Author: fhanik
>> Date: Thu May 10 04:30:29 2007
>> New Revision: 536830
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=536830
>> Log:
>> New proposed API calls for Comet
>> +    /**
>> +     * notification bit to receive an event
>> +     * immediately, the event will be NOTIFY/WRITE
>> +     * @see #register(int)
>> +     */
>> +    public static final int OP_NOW = 0x001;
>> +    +    /**
>> +     * notification bit to receive an event when
>> +     * data has arrived on the connection
>> +     * Event will be READ
>> +     * @see #register(int)
>> +     * @see CometProcessor#event(CometEvent)
>> +     */
>> +    public static final int OP_READ = 0x004;
>
> Besides the cute boolean tricks that are soooo cool, I think an enum 
> should be used.
No problem, enum with a vararg works just fine
>
>> +    public void register(int notificationOptions)
>> +        throws IOException, ServletException, IllegalStateException;
>
> The throws list for all these methods is a bit abusive. IOException, 
> maybe it would be useful (but I will remove it if it's not), but 
> ServletException ??
I agree, ServletException should go, IOException, I'd leave it in, ie, 
you don't break the API if you remove it later, but you do break the API 
if you have to add it later
>
> I am ok with proceeding based on that, but I'll likely ask for 
> adjustment of some behaviors. For example, I am not convinced with the 
> registration for read after each read event (it could maybe cause a 
> leak like the one I fixed recently; I think it could be avoided, but 
> this requires passing around flags, which is a bit annoying).
I hadn't planned that you need to re-register for a READ event. 
Essentially, after an event processed in CometProcessor.event, the 
registrations that you had before will stand.
>
> > there is one inconsistency,
> > currently READ is a main type, while WRITE is a subtype of NOTIFY
> > thoughts on consolidate these? ie NOTIFY/READ or make WRITE a main type
>
> If you think about NOTIFY as a name, then all events could be 
> NOTIFY/something. Since this is not very practical, I would not be 
> using a NOTIFY in the event type.
:) totally agree, any thoughts on READ vs WRITE, should WRITE be a main 
type?
>
> Rémy
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message