You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Emmanuel Lecharny <el...@apache.org> on 2008/06/16 22:54:58 UTC
Removing some useless interfaces
Hi gang,
I have recently reviewed some part of the IoSession inheritence tree,
and discovered that some interfaces are just useless :
- SocketSession
- DatagramSession
I suggest we remove them (it won't harm the project or the tests in any
case).
Anyone see a reason to keep those useless and undocumented interfaces ?
Thanks !
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
Re: Removing some useless interfaces
Posted by Julien Vermillard <jv...@archean.fr>.
On Mon, 16 Jun 2008 22:54:58 +0200
Emmanuel Lecharny <el...@apache.org> wrote:
> Hi gang,
>
> I have recently reviewed some part of the IoSession inheritence tree,
> and discovered that some interfaces are just useless :
> - SocketSession
> - DatagramSession
>
> I suggest we remove them (it won't harm the project or the tests in
> any case).
>
> Anyone see a reason to keep those useless and undocumented
> interfaces ?
>
> Thanks !
>
Hi,
As far I understand it, they are there for having the good type returned
when you call getLocalAddress and getRemoteAddress.
Cause most of time we use IoSession typed sessions, I don't think it's
very useful, you need to cast the IoSession into the correct type for
having the good getXXAddress return type.
ex :
IoSession session = connectFuture.getSession();
InetSocketAddress address =
((InetSocketSession)session).getLocalAddress();
So we can cast the result of getXXXAddress directly and remove those
interfaces.
IoSession session = connectFuture.getSession();
InetSocketAddress address =
(InetSocketAddress)session.getLocalAddress();
For DatagramService and SocketService look like it's the case too.
I think the hierarchy tree look like a bit over-engineered, but it's
perhaps my taste for simple things ;)
Julien
Re: Removing some useless interfaces
Posted by Julien Vermillard <jv...@archean.fr>.
On Thu, 19 Jun 2008 10:04:47 +0200
Niklas Therning <ni...@trillian.se> wrote:
> Julien Vermillard skrev:
> > On Mon, 16 Jun 2008 22:54:58 +0200
> > Emmanuel Lecharny <el...@apache.org> wrote:
> >
> >
> >> Hi gang,
> >>
> >> I have recently reviewed some part of the IoSession inheritence
> >> tree, and discovered that some interfaces are just useless :
> >> - SocketSession
> >> - DatagramSession
> >>
> >> I suggest we remove them (it won't harm the project or the tests in
> >> any case).
> >>
> >> Anyone see a reason to keep those useless and undocumented
> >> interfaces ?
> >>
> >> Thanks !
> >>
> >>
> >
> > I found a side effect :
> >
> > SocketConnector connector = new NioSocketConnector();
> > connector.setHandler(handler);
> > connector.setConnectTimeoutMillis(2000);
> > connector.getSessionConfig().setReaderIdleTime(10);
> > connector.getSessionConfig().setWriterIdleTime(10);
> > In place of writing :
> > connector.getSessionConfig().setTcpNoDelay(true);
> > You need to cast like that :
> > ((SocketSessionConfig)connector.getSessionConfig()).setTcpNoDelay(true);
> >
> > Julien
> I think this is exactly why these interfaces were introduced, to be
> able to use Java5 covarient return types. I wouldn't say they are
> useless because they obviously save you from doing casts like this.
>
> /Niklas
>
I'm a bit daft this morning, I can't find any coffee,
I commited a fix for that, SocketAcceptor and SocketConnector override
getSessionConfig with the good IoSessionConfig subclass.
http://svn.apache.org/viewvc?view=rev&revision=669418
Julien
Re: Removing some useless interfaces
Posted by Niklas Therning <ni...@trillian.se>.
Julien Vermillard skrev:
> On Thu, 19 Jun 2008 10:56:11 +0200
> Niklas Therning <ni...@trillian.se> wrote:
>
>
>> Ok, you're right, it's a bit overkill to have an interface just to
>> avoid a cast. But SocketService still had its right to exist I think.
>> What if you want to explicitly depend on an IoService which handles
>> socket based transports? Removing SocketService means that one would
>> need to use IoService instead and then do the type checking at
>> runtime rather than at compile time.
>>
>> /Niklas
>>
>
> You still got SocketAcceptor and SocketConnector, I don't see much
> interest in a general SocketService iface with 2 methods.
>
> I think you need to depend on a Socket based acceptor or
> connector, not a SocketService no ?
>
> Julien
>
You're probably right.
/Niklas
Re: Removing some useless interfaces
Posted by Julien Vermillard <jv...@archean.fr>.
On Thu, 19 Jun 2008 10:56:11 +0200
Niklas Therning <ni...@trillian.se> wrote:
> Ok, you're right, it's a bit overkill to have an interface just to
> avoid a cast. But SocketService still had its right to exist I think.
> What if you want to explicitly depend on an IoService which handles
> socket based transports? Removing SocketService means that one would
> need to use IoService instead and then do the type checking at
> runtime rather than at compile time.
>
> /Niklas
You still got SocketAcceptor and SocketConnector, I don't see much
interest in a general SocketService iface with 2 methods.
I think you need to depend on a Socket based acceptor or
connector, not a SocketService no ?
Julien
Re: Removing some useless interfaces
Posted by Emmanuel Lecharny <el...@apache.org>.
Niklas Therning wrote:
> Emmanuel Lecharny skrev:
>> Niklas Therning wrote:
>>> Julien Vermillard skrev:
>>>> On Mon, 16 Jun 2008 22:54:58 +0200
>>>> Emmanuel Lecharny <el...@apache.org> wrote:
>>>>
>>>>
>>>>> Hi gang,
>>>>>
>>>>> I have recently reviewed some part of the IoSession inheritence
>>>>> tree, and discovered that some interfaces are just useless :
>>>>> - SocketSession
>>>>> - DatagramSession
>>>>>
>>>>> I suggest we remove them (it won't harm the project or the tests in
>>>>> any case).
>>>>>
>>>>> Anyone see a reason to keep those useless and undocumented
>>>>> interfaces ?
>>>>>
>>>>> Thanks !
>>>>>
>>>>>
>>>>
>>>> I found a side effect :
>>>>
>>>> SocketConnector connector = new NioSocketConnector();
>>>> connector.setHandler(handler);
>>>> connector.setConnectTimeoutMillis(2000);
>>>> connector.getSessionConfig().setReaderIdleTime(10);
>>>> connector.getSessionConfig().setWriterIdleTime(10);
>>>> In place of writing :
>>>> connector.getSessionConfig().setTcpNoDelay(true);
>>>> You need to cast like that :
>>>> ((SocketSessionConfig)connector.getSessionConfig()).setTcpNoDelay(true);
>>>>
>>>>
>>>> Julien
>>> I think this is exactly why these interfaces were introduced, to be
>>> able to use Java5 covarient return types. I wouldn't say they are
>>> useless because they obviously save you from doing casts like this.
>> IMHO, introducing an interface for the sole purpose of avoiding a
>> cast is a little bit overkilling. Would it be more than one cast, I
>> would agree, but otherwise, I think we have other options, like
>> declaring the constructed element as in instance of the class, and
>> not as an "instance" of the interface. Something like :
>>
>> // Connector is not declared as a Socketonnector
>> NioSocketConnector connector = new NioSocketConnector();
>> blah ...
>> connector.getSessionConfig().setTcpNoDelay(true);
>>
>> work just fine _and_ does not need to have an interface to be defined.
>>
>> wdyt ?
>>
>>
> Ok, you're right, it's a bit overkill to have an interface just to
> avoid a cast. But SocketService still had its right to exist I think.
> What if you want to explicitly depend on an IoService which handles
> socket based transports? Removing SocketService means that one would
> need to use IoService instead and then do the type checking at runtime
> rather than at compile time.
The thing is that unless this need really exists, we would better keep
the inheritence scheme as simple as possible. The previous
Interface/Class inheritence scheme was a nightmare, with many levels and
also useless implementations of interfaces which are already implemented
by some extended classes.
Regardless to these design discussion, in your use case, as the
NioSocketConnector/Acceptor indirectly implements the IoService
interface, everything will work pristine. Remember that the IoService
interface was just containing an accessor to some SessinConfig object,
and Julien has added the accessor into the SocketConnector/Acceptor
interfaces.
I really think we are safe.
Otherwise, as stated by Julien, those modifications had some impact on
AsyncWeb (and may be on FtpServer). I will pot a mail about that.
Thanks !
>
> /Niklas
>
>
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
Re: Removing some useless interfaces
Posted by Niklas Therning <ni...@trillian.se>.
Emmanuel Lecharny skrev:
> Niklas Therning wrote:
>> Julien Vermillard skrev:
>>> On Mon, 16 Jun 2008 22:54:58 +0200
>>> Emmanuel Lecharny <el...@apache.org> wrote:
>>>
>>>
>>>> Hi gang,
>>>>
>>>> I have recently reviewed some part of the IoSession inheritence
>>>> tree, and discovered that some interfaces are just useless :
>>>> - SocketSession
>>>> - DatagramSession
>>>>
>>>> I suggest we remove them (it won't harm the project or the tests in
>>>> any case).
>>>>
>>>> Anyone see a reason to keep those useless and undocumented
>>>> interfaces ?
>>>>
>>>> Thanks !
>>>>
>>>>
>>>
>>> I found a side effect :
>>>
>>> SocketConnector connector = new NioSocketConnector();
>>> connector.setHandler(handler);
>>> connector.setConnectTimeoutMillis(2000);
>>> connector.getSessionConfig().setReaderIdleTime(10);
>>> connector.getSessionConfig().setWriterIdleTime(10);
>>> In place of writing :
>>> connector.getSessionConfig().setTcpNoDelay(true);
>>> You need to cast like that :
>>> ((SocketSessionConfig)connector.getSessionConfig()).setTcpNoDelay(true);
>>>
>>>
>>> Julien
>> I think this is exactly why these interfaces were introduced, to be
>> able to use Java5 covarient return types. I wouldn't say they are
>> useless because they obviously save you from doing casts like this.
> IMHO, introducing an interface for the sole purpose of avoiding a cast
> is a little bit overkilling. Would it be more than one cast, I would
> agree, but otherwise, I think we have other options, like declaring
> the constructed element as in instance of the class, and not as an
> "instance" of the interface. Something like :
>
> // Connector is not declared as a Socketonnector
> NioSocketConnector connector = new NioSocketConnector();
> blah ...
> connector.getSessionConfig().setTcpNoDelay(true);
>
> work just fine _and_ does not need to have an interface to be defined.
>
> wdyt ?
>
>
Ok, you're right, it's a bit overkill to have an interface just to avoid
a cast. But SocketService still had its right to exist I think. What if
you want to explicitly depend on an IoService which handles socket based
transports? Removing SocketService means that one would need to use
IoService instead and then do the type checking at runtime rather than
at compile time.
/Niklas
Re: Removing some useless interfaces
Posted by Emmanuel Lecharny <el...@apache.org>.
Niklas Therning wrote:
> Julien Vermillard skrev:
>> On Mon, 16 Jun 2008 22:54:58 +0200
>> Emmanuel Lecharny <el...@apache.org> wrote:
>>
>>
>>> Hi gang,
>>>
>>> I have recently reviewed some part of the IoSession inheritence
>>> tree, and discovered that some interfaces are just useless :
>>> - SocketSession
>>> - DatagramSession
>>>
>>> I suggest we remove them (it won't harm the project or the tests in
>>> any case).
>>>
>>> Anyone see a reason to keep those useless and undocumented
>>> interfaces ?
>>>
>>> Thanks !
>>>
>>>
>>
>> I found a side effect :
>>
>> SocketConnector connector = new NioSocketConnector();
>> connector.setHandler(handler);
>> connector.setConnectTimeoutMillis(2000);
>> connector.getSessionConfig().setReaderIdleTime(10);
>> connector.getSessionConfig().setWriterIdleTime(10);
>> In place of writing :
>> connector.getSessionConfig().setTcpNoDelay(true);
>> You need to cast like that :
>> ((SocketSessionConfig)connector.getSessionConfig()).setTcpNoDelay(true);
>>
>> Julien
> I think this is exactly why these interfaces were introduced, to be
> able to use Java5 covarient return types. I wouldn't say they are
> useless because they obviously save you from doing casts like this.
IMHO, introducing an interface for the sole purpose of avoiding a cast
is a little bit overkilling. Would it be more than one cast, I would
agree, but otherwise, I think we have other options, like declaring the
constructed element as in instance of the class, and not as an
"instance" of the interface. Something like :
// Connector is not declared as a Socketonnector
NioSocketConnector connector = new NioSocketConnector();
blah ...
connector.getSessionConfig().setTcpNoDelay(true);
work just fine _and_ does not need to have an interface to be defined.
wdyt ?
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
Re: Removing some useless interfaces
Posted by Niklas Therning <ni...@trillian.se>.
Julien Vermillard skrev:
> On Mon, 16 Jun 2008 22:54:58 +0200
> Emmanuel Lecharny <el...@apache.org> wrote:
>
>
>> Hi gang,
>>
>> I have recently reviewed some part of the IoSession inheritence tree,
>> and discovered that some interfaces are just useless :
>> - SocketSession
>> - DatagramSession
>>
>> I suggest we remove them (it won't harm the project or the tests in
>> any case).
>>
>> Anyone see a reason to keep those useless and undocumented
>> interfaces ?
>>
>> Thanks !
>>
>>
>
> I found a side effect :
>
> SocketConnector connector = new NioSocketConnector();
> connector.setHandler(handler);
> connector.setConnectTimeoutMillis(2000);
> connector.getSessionConfig().setReaderIdleTime(10);
> connector.getSessionConfig().setWriterIdleTime(10);
> In place of writing :
> connector.getSessionConfig().setTcpNoDelay(true);
> You need to cast like that :
> ((SocketSessionConfig)connector.getSessionConfig()).setTcpNoDelay(true);
>
> Julien
I think this is exactly why these interfaces were introduced, to be able
to use Java5 covarient return types. I wouldn't say they are useless
because they obviously save you from doing casts like this.
/Niklas
Re: Removing some useless interfaces
Posted by Julien Vermillard <jv...@archean.fr>.
On Mon, 16 Jun 2008 22:54:58 +0200
Emmanuel Lecharny <el...@apache.org> wrote:
> Hi gang,
>
> I have recently reviewed some part of the IoSession inheritence tree,
> and discovered that some interfaces are just useless :
> - SocketSession
> - DatagramSession
>
> I suggest we remove them (it won't harm the project or the tests in
> any case).
>
> Anyone see a reason to keep those useless and undocumented
> interfaces ?
>
> Thanks !
>
I found a side effect :
SocketConnector connector = new NioSocketConnector();
connector.setHandler(handler);
connector.setConnectTimeoutMillis(2000);
connector.getSessionConfig().setReaderIdleTime(10);
connector.getSessionConfig().setWriterIdleTime(10);
In place of writing :
connector.getSessionConfig().setTcpNoDelay(true);
You need to cast like that :
((SocketSessionConfig)connector.getSessionConfig()).setTcpNoDelay(true);
Julien