Maarten Bosteels wrote:
On Feb 5, 2008 11:07 AM, Emmanuel Lecharny <[EMAIL PROTECTED]> wrote:
Maarten Bosteels wrote:
I agree that nobody will pass in hours or days, but IMHO it improves
readability a lot:
connector.setConnectTimeout(250, TimeUnit.MILLISECONDS);
or
connector.setConnectTimeout(250);
Especially when one is changing the API from seconds to millis, we
should
try to make
it unambiguous and it's inline with a lot of the java.util.concurrentAPI.
The problem with changing the method parameters semantic is that it does
not align anymore with the underlying semantic of the select( timeout )
method :
select
public abstract int *select*(long timeout)
throws IOException <
http://java.sun.com/j2se/1.4.2/docs/api/java/io/IOException.html>
...
*Parameters:*
|timeout| - If positive, block for up to timeout milliseconds, more
or less...
The method should use milliseconds, no need to be explicit with that,
IMHO. I agree with Julien on that.
ok, I can live with that.
But you wouldn't even change the metod name ?
So users who are porting there code from mina 1.x to 2.x
will have to be very careful, or their timeouts will shrink with a factor of
1000.
Maarten
You would also need to make sure that the current IoConnector
implementations can support millisecond timeouts. I think that would
mean that AbstractPollingIoConnector.Worker needs to be changed.
Specifically the line
boolean selected = select(1000);
To support milliseconds we could simply change this into
boolean selected = select(1);
but that would be very bad for performance.
Instead, one could use an adaptive timeout in the call to select() which
depends on the current ConnectRequests' timeouts. Or totally redesign
things to use a ScheduledExecutorService or similar instead.
--
Niklas Therning
www.spamdrain.net
