Re: AbstractIoSession and final qualifiers
jgenender wrote: Hey guys, I was hoping to see if we could discuss some of the final qualifiers on some of the methods in the AbstractIOSession. The reason I ask is it would be cool to be able to override some of the methods such as: public WriteFuture write(Object message) public final int getScheduledWriteMessages() I got the same problem some time ago. I resolved it with an IoFilter but I'm not very happy with it, because the code should be be like: call method X and expect it to call IoSession.write with object Y but it is like: call method X and expect it to call (IoSession.write which has to call) IoFilter.filterWrite with a WriteRequest with message object Y while the part in braces is implicit and therefore not intuitive. jgenender wrote: To be able to write MockObjects for unit tests of code. Any thoughts on making these protected instead of final? No, protected is wrong. I want to call the write method from another class in another package so it must be public. ;-) regards Steve -- View this message in context: http://www.nabble.com/AbstractIoSession-and-final-qualifiers-tp14507695s16868p14574837.html Sent from the Apache MINA Support Forum mailing list archive at Nabble.com.
Re: AbstractIoSession and final qualifiers
Steve Ulrich (proemion) wrote: Emmanuel Lecharny-3 wrote: Wouldn't be a perfect case for an assert ? Like : public final WriteFuture write(Object message, SocketAddress remoteAddress) { assert message != null : Null messages are not allowed; ... wdyt ? NO! You are right, I'm wrong, and we already discussed that with Trustin. I just picked the wrong example from the code. The fact that this method is public just slipped oiut of my attention : public method must NOT use assertion to check for arguments. And in this very case, Trustin said that the user may want to get a NPE which will be more easy to manage from the user perspective. Consider this thread as a dead end and my suggestion as wrong practice. Thanks for the heads up, Steave, and Happy New Year too ! btw, wishes += Do not work too late and post stupid proposals to the mailing list at 1 AM... -- -- cordialement, regards, Emmanuel Lécharny www.iktek.com directory.apache.org
Re: AbstractIoSession and final qualifiers
Emmanuel Lecharny-3 wrote: While looking at the abstract class, I found some methods with this kind of code : public final WriteFuture write(Object message, SocketAddress remoteAddress) { if (message == null) { throw new NullPointerException(message); } ... Wouldn't be a perfect case for an assert ? Like : public final WriteFuture write(Object message, SocketAddress remoteAddress) { assert message != null : Null messages are not allowed; ... wdyt ? NO! There are recommendations from SUN, which tells you where to use assert and where not. Method arguments checking per assert is definitive a no-go! http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html#usage http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html#usage (See first point!) regards and a happy new year, Steve -- View this message in context: http://www.nabble.com/AbstractIoSession-and-final-qualifiers-tp14507695s16868p14574728.html Sent from the Apache MINA Support Forum mailing list archive at Nabble.com.
Re: AbstractIoSession and final qualifiers
Trustin (and others)...so how about these changes and additions to the AbstractIoSession? This will allow the scheduledBytes/messages to be overriden, and also allow someone to set them. Index: core/src/main/java/org/apache/mina/common/AbstractIoSession.java === --- core/src/main/java/org/apache/mina/common/AbstractIoSession.java (revision 607135) +++ core/src/main/java/org/apache/mina/common/AbstractIoSession.java (working copy) @@ -488,14 +488,22 @@ lastThroughputCalculationTime = currentTime; } -public final long getScheduledWriteBytes() { +public long getScheduledWriteBytes() { return scheduledWriteBytes.get(); } -public final int getScheduledWriteMessages() { +public int getScheduledWriteMessages() { return scheduledWriteMessages.get(); } +public void setScheduledWriteBytes(long byteCount){ +scheduledWriteBytes.set(byteCount); +} + +public void setScheduledWriteMessages(int messages) { +scheduledWriteMessages.set(messages); +} + protected final void increaseReadBytes(long increment, long currentTime) { if (increment = 0) { return; Thanks, Jeff Trustin Lee wrote: Hi Jeff and Emmanuel, You can add some hook for IoSession.write() by inserting an IoFilter, so I am not sure about removing the final modifier from write(). Overriding getScheduledMessages and getScheduledBytes makes sense though because they will always be 0 because messageSent event will be always fired immediately. WDYT? Trustin On Dec 27, 2007 10:21 AM, Emmanuel Lecharny [EMAIL PROTECTED] wrote: Jeff Genender wrote: What do *you* think? ;-) Beside the assert, which was a side effect of my quick glance at the method you want to override, I think that dooming the final is sane. If someone needs to protect this method, then the best solution would be to define a super class with final methods calling the AbstractIoSession non final methods. Another solution (puke) would be to remove the public/protected/private qualifier, and to add a MockIoSession in the same package. Not very elegant, but does the job too... There are always tradeoff when you want to thoroughly unit-test some code. When it comes to an API, it seems impossible to avoid those tradeoff, IMHO. Any other opinion ? Trustin ? Julien ? Jeff Emmanuel Lecharny wrote: Jeff Genender wrote: Hey guys, I was hoping to see if we could discuss some of the final qualifiers on some of the methods in the AbstractIOSession. The reason I ask is it would be cool to be able to override some of the methods such as: public WriteFuture write(Object message) public final int getScheduledWriteMessages() To be able to write MockObjects for unit tests of code. Any thoughts on making these protected instead of final? Thanks, Jeff Hi, makes sense to me... Or we may have a AbstractMockIoSession implementing the IoSession interface, for unit tests ? While looking at the abstract class, I found some methods with this kind of code : public final WriteFuture write(Object message, SocketAddress remoteAddress) { if (message == null) { throw new NullPointerException(message); } ... Wouldn't be a perfect case for an assert ? Like : public final WriteFuture write(Object message, SocketAddress remoteAddress) { assert message != null : Null messages are not allowed; ... wdyt ? -- -- cordialement, regards, Emmanuel Lécharny www.iktek.com directory.apache.org
Re: AbstractIoSession and final qualifiers
Hi Jeff, Do we need to remove the final modifier if we are going to provide setters? And assuming you are using DummySession as a mock object, I'd suggest making AbstractIoSession.setScheduled...(...) protected and make them public in DummySession. WDYT? Cheers, Trustin On Dec 28, 2007 8:19 AM, Jeff Genender [EMAIL PROTECTED] wrote: Trustin (and others)...so how about these changes and additions to the AbstractIoSession? This will allow the scheduledBytes/messages to be overriden, and also allow someone to set them. Index: core/src/main/java/org/apache/mina/common/AbstractIoSession.java === --- core/src/main/java/org/apache/mina/common/AbstractIoSession.java (revision 607135) +++ core/src/main/java/org/apache/mina/common/AbstractIoSession.java (working copy) @@ -488,14 +488,22 @@ lastThroughputCalculationTime = currentTime; } -public final long getScheduledWriteBytes() { +public long getScheduledWriteBytes() { return scheduledWriteBytes.get(); } -public final int getScheduledWriteMessages() { +public int getScheduledWriteMessages() { return scheduledWriteMessages.get(); } +public void setScheduledWriteBytes(long byteCount){ +scheduledWriteBytes.set(byteCount); +} + +public void setScheduledWriteMessages(int messages) { +scheduledWriteMessages.set(messages); +} + protected final void increaseReadBytes(long increment, long currentTime) { if (increment = 0) { return; Thanks, Jeff Trustin Lee wrote: Hi Jeff and Emmanuel, You can add some hook for IoSession.write() by inserting an IoFilter, so I am not sure about removing the final modifier from write(). Overriding getScheduledMessages and getScheduledBytes makes sense though because they will always be 0 because messageSent event will be always fired immediately. WDYT? Trustin On Dec 27, 2007 10:21 AM, Emmanuel Lecharny [EMAIL PROTECTED] wrote: Jeff Genender wrote: What do *you* think? ;-) Beside the assert, which was a side effect of my quick glance at the method you want to override, I think that dooming the final is sane. If someone needs to protect this method, then the best solution would be to define a super class with final methods calling the AbstractIoSession non final methods. Another solution (puke) would be to remove the public/protected/private qualifier, and to add a MockIoSession in the same package. Not very elegant, but does the job too... There are always tradeoff when you want to thoroughly unit-test some code. When it comes to an API, it seems impossible to avoid those tradeoff, IMHO. Any other opinion ? Trustin ? Julien ? Jeff Emmanuel Lecharny wrote: Jeff Genender wrote: Hey guys, I was hoping to see if we could discuss some of the final qualifiers on some of the methods in the AbstractIOSession. The reason I ask is it would be cool to be able to override some of the methods such as: public WriteFuture write(Object message) public final int getScheduledWriteMessages() To be able to write MockObjects for unit tests of code. Any thoughts on making these protected instead of final? Thanks, Jeff Hi, makes sense to me... Or we may have a AbstractMockIoSession implementing the IoSession interface, for unit tests ? While looking at the abstract class, I found some methods with this kind of code : public final WriteFuture write(Object message, SocketAddress remoteAddress) { if (message == null) { throw new NullPointerException(message); } ... Wouldn't be a perfect case for an assert ? Like : public final WriteFuture write(Object message, SocketAddress remoteAddress) { assert message != null : Null messages are not allowed; ... wdyt ? -- -- cordialement, regards, Emmanuel Lécharny www.iktek.com directory.apache.org -- what we call human nature is actually human habit -- http://gleamynode.net/ -- PGP Key ID: 0x0255ECA6
Re: AbstractIoSession and final qualifiers
Yep...agreed...how is this? Index: core/src/main/java/org/apache/mina/common/AbstractIoSession.java === --- core/src/main/java/org/apache/mina/common/AbstractIoSession.java (revision 607135) +++ core/src/main/java/org/apache/mina/common/AbstractIoSession.java (working copy) @@ -496,6 +496,14 @@ return scheduledWriteMessages.get(); } +protected void setScheduledWriteBytes(long byteCount){ +scheduledWriteBytes.set(byteCount); +} + +protected void setScheduledWriteMessages(int messages) { +scheduledWriteMessages.set(messages); +} + protected final void increaseReadBytes(long increment, long currentTime) { if (increment = 0) { return; Trustin Lee wrote: Hi Jeff, Do we need to remove the final modifier if we are going to provide setters? And assuming you are using DummySession as a mock object, I'd suggest making AbstractIoSession.setScheduled...(...) protected and make them public in DummySession. WDYT? Cheers, Trustin On Dec 28, 2007 8:19 AM, Jeff Genender [EMAIL PROTECTED] wrote: Trustin (and others)...so how about these changes and additions to the AbstractIoSession? This will allow the scheduledBytes/messages to be overriden, and also allow someone to set them. Index: core/src/main/java/org/apache/mina/common/AbstractIoSession.java === --- core/src/main/java/org/apache/mina/common/AbstractIoSession.java (revision 607135) +++ core/src/main/java/org/apache/mina/common/AbstractIoSession.java (working copy) @@ -488,14 +488,22 @@ lastThroughputCalculationTime = currentTime; } -public final long getScheduledWriteBytes() { +public long getScheduledWriteBytes() { return scheduledWriteBytes.get(); } -public final int getScheduledWriteMessages() { +public int getScheduledWriteMessages() { return scheduledWriteMessages.get(); } +public void setScheduledWriteBytes(long byteCount){ +scheduledWriteBytes.set(byteCount); +} + +public void setScheduledWriteMessages(int messages) { +scheduledWriteMessages.set(messages); +} + protected final void increaseReadBytes(long increment, long currentTime) { if (increment = 0) { return; Thanks, Jeff Trustin Lee wrote: Hi Jeff and Emmanuel, You can add some hook for IoSession.write() by inserting an IoFilter, so I am not sure about removing the final modifier from write(). Overriding getScheduledMessages and getScheduledBytes makes sense though because they will always be 0 because messageSent event will be always fired immediately. WDYT? Trustin On Dec 27, 2007 10:21 AM, Emmanuel Lecharny [EMAIL PROTECTED] wrote: Jeff Genender wrote: What do *you* think? ;-) Beside the assert, which was a side effect of my quick glance at the method you want to override, I think that dooming the final is sane. If someone needs to protect this method, then the best solution would be to define a super class with final methods calling the AbstractIoSession non final methods. Another solution (puke) would be to remove the public/protected/private qualifier, and to add a MockIoSession in the same package. Not very elegant, but does the job too... There are always tradeoff when you want to thoroughly unit-test some code. When it comes to an API, it seems impossible to avoid those tradeoff, IMHO. Any other opinion ? Trustin ? Julien ? Jeff Emmanuel Lecharny wrote: Jeff Genender wrote: Hey guys, I was hoping to see if we could discuss some of the final qualifiers on some of the methods in the AbstractIOSession. The reason I ask is it would be cool to be able to override some of the methods such as: public WriteFuture write(Object message) public final int getScheduledWriteMessages() To be able to write MockObjects for unit tests of code. Any thoughts on making these protected instead of final? Thanks, Jeff Hi, makes sense to me... Or we may have a AbstractMockIoSession implementing the IoSession interface, for unit tests ? While looking at the abstract class, I found some methods with this kind of code : public final WriteFuture write(Object message, SocketAddress remoteAddress) { if (message == null) { throw new NullPointerException(message); } ... Wouldn't be a perfect case for an assert ? Like : public final WriteFuture write(Object message, SocketAddress remoteAddress) { assert message != null : Null messages are not allowed; ... wdyt ? -- -- cordialement, regards, Emmanuel Lécharny www.iktek.com directory.apache.org
Re: AbstractIoSession and final qualifiers
I like it (the assert) ;-) But I am not sure about having a completely different AbstractIOMockSession since we would then be copying code to a certain degree. I think it would just be cleaner to have the methods protected so there is no need for code duplication ;-) What do *you* think? ;-) Jeff Emmanuel Lecharny wrote: Jeff Genender wrote: Hey guys, I was hoping to see if we could discuss some of the final qualifiers on some of the methods in the AbstractIOSession. The reason I ask is it would be cool to be able to override some of the methods such as: public WriteFuture write(Object message) public final int getScheduledWriteMessages() To be able to write MockObjects for unit tests of code. Any thoughts on making these protected instead of final? Thanks, Jeff Hi, makes sense to me... Or we may have a AbstractMockIoSession implementing the IoSession interface, for unit tests ? While looking at the abstract class, I found some methods with this kind of code : public final WriteFuture write(Object message, SocketAddress remoteAddress) { if (message == null) { throw new NullPointerException(message); } ... Wouldn't be a perfect case for an assert ? Like : public final WriteFuture write(Object message, SocketAddress remoteAddress) { assert message != null : Null messages are not allowed; ... wdyt ?
Re: AbstractIoSession and final qualifiers
Jeff Genender wrote: What do *you* think? ;-) Beside the assert, which was a side effect of my quick glance at the method you want to override, I think that dooming the final is sane. If someone needs to protect this method, then the best solution would be to define a super class with final methods calling the AbstractIoSession non final methods. Another solution (puke) would be to remove the public/protected/private qualifier, and to add a MockIoSession in the same package. Not very elegant, but does the job too... There are always tradeoff when you want to thoroughly unit-test some code. When it comes to an API, it seems impossible to avoid those tradeoff, IMHO. Any other opinion ? Trustin ? Julien ? Jeff Emmanuel Lecharny wrote: Jeff Genender wrote: Hey guys, I was hoping to see if we could discuss some of the final qualifiers on some of the methods in the AbstractIOSession. The reason I ask is it would be cool to be able to override some of the methods such as: public WriteFuture write(Object message) public final int getScheduledWriteMessages() To be able to write MockObjects for unit tests of code. Any thoughts on making these protected instead of final? Thanks, Jeff Hi, makes sense to me... Or we may have a AbstractMockIoSession implementing the IoSession interface, for unit tests ? While looking at the abstract class, I found some methods with this kind of code : public final WriteFuture write(Object message, SocketAddress remoteAddress) { if (message == null) { throw new NullPointerException(message); } ... Wouldn't be a perfect case for an assert ? Like : public final WriteFuture write(Object message, SocketAddress remoteAddress) { assert message != null : Null messages are not allowed; ... wdyt ? -- -- cordialement, regards, Emmanuel Lécharny www.iktek.com directory.apache.org
Re: AbstractIoSession and final qualifiers
Hi Jeff and Emmanuel, You can add some hook for IoSession.write() by inserting an IoFilter, so I am not sure about removing the final modifier from write(). Overriding getScheduledMessages and getScheduledBytes makes sense though because they will always be 0 because messageSent event will be always fired immediately. WDYT? Trustin On Dec 27, 2007 10:21 AM, Emmanuel Lecharny [EMAIL PROTECTED] wrote: Jeff Genender wrote: What do *you* think? ;-) Beside the assert, which was a side effect of my quick glance at the method you want to override, I think that dooming the final is sane. If someone needs to protect this method, then the best solution would be to define a super class with final methods calling the AbstractIoSession non final methods. Another solution (puke) would be to remove the public/protected/private qualifier, and to add a MockIoSession in the same package. Not very elegant, but does the job too... There are always tradeoff when you want to thoroughly unit-test some code. When it comes to an API, it seems impossible to avoid those tradeoff, IMHO. Any other opinion ? Trustin ? Julien ? Jeff Emmanuel Lecharny wrote: Jeff Genender wrote: Hey guys, I was hoping to see if we could discuss some of the final qualifiers on some of the methods in the AbstractIOSession. The reason I ask is it would be cool to be able to override some of the methods such as: public WriteFuture write(Object message) public final int getScheduledWriteMessages() To be able to write MockObjects for unit tests of code. Any thoughts on making these protected instead of final? Thanks, Jeff Hi, makes sense to me... Or we may have a AbstractMockIoSession implementing the IoSession interface, for unit tests ? While looking at the abstract class, I found some methods with this kind of code : public final WriteFuture write(Object message, SocketAddress remoteAddress) { if (message == null) { throw new NullPointerException(message); } ... Wouldn't be a perfect case for an assert ? Like : public final WriteFuture write(Object message, SocketAddress remoteAddress) { assert message != null : Null messages are not allowed; ... wdyt ? -- -- cordialement, regards, Emmanuel Lécharny www.iktek.com directory.apache.org -- what we call human nature is actually human habit -- http://gleamynode.net/ -- PGP Key ID: 0x0255ECA6
Re: AbstractIoSession and final qualifiers
Alternatively, we could add a protected method in AbstractIoSession and make write() method to call it before returning. However, it is essentially the same with adding an IoFilter IMHO. The implementation of IoSession and IoFilterChain is not so trivial so creating another mock object is a kind of huge task so we will finally have a lot of duplicated code there, so I think it is somewhat difficult to create a separate mock object; I already tried to do that and gave up. :) Trustin On Dec 27, 2007 10:48 AM, Trustin Lee [EMAIL PROTECTED] wrote: Hi Jeff and Emmanuel, You can add some hook for IoSession.write() by inserting an IoFilter, so I am not sure about removing the final modifier from write(). Overriding getScheduledMessages and getScheduledBytes makes sense though because they will always be 0 because messageSent event will be always fired immediately. WDYT? Trustin On Dec 27, 2007 10:21 AM, Emmanuel Lecharny [EMAIL PROTECTED] wrote: Jeff Genender wrote: What do *you* think? ;-) Beside the assert, which was a side effect of my quick glance at the method you want to override, I think that dooming the final is sane. If someone needs to protect this method, then the best solution would be to define a super class with final methods calling the AbstractIoSession non final methods. Another solution (puke) would be to remove the public/protected/private qualifier, and to add a MockIoSession in the same package. Not very elegant, but does the job too... There are always tradeoff when you want to thoroughly unit-test some code. When it comes to an API, it seems impossible to avoid those tradeoff, IMHO. Any other opinion ? Trustin ? Julien ? Jeff Emmanuel Lecharny wrote: Jeff Genender wrote: Hey guys, I was hoping to see if we could discuss some of the final qualifiers on some of the methods in the AbstractIOSession. The reason I ask is it would be cool to be able to override some of the methods such as: public WriteFuture write(Object message) public final int getScheduledWriteMessages() To be able to write MockObjects for unit tests of code. Any thoughts on making these protected instead of final? Thanks, Jeff Hi, makes sense to me... Or we may have a AbstractMockIoSession implementing the IoSession interface, for unit tests ? While looking at the abstract class, I found some methods with this kind of code : public final WriteFuture write(Object message, SocketAddress remoteAddress) { if (message == null) { throw new NullPointerException(message); } ... Wouldn't be a perfect case for an assert ? Like : public final WriteFuture write(Object message, SocketAddress remoteAddress) { assert message != null : Null messages are not allowed; ... wdyt ? -- -- cordialement, regards, Emmanuel Lécharny www.iktek.com directory.apache.org -- what we call human nature is actually human habit -- http://gleamynode.net/ -- PGP Key ID: 0x0255ECA6 -- what we call human nature is actually human habit -- http://gleamynode.net/ -- PGP Key ID: 0x0255ECA6
Re: AbstractIoSession and final qualifiers
I agree...this makes sense...Filter is the way to go. Jeff Trustin Lee wrote: Alternatively, we could add a protected method in AbstractIoSession and make write() method to call it before returning. However, it is essentially the same with adding an IoFilter IMHO. The implementation of IoSession and IoFilterChain is not so trivial so creating another mock object is a kind of huge task so we will finally have a lot of duplicated code there, so I think it is somewhat difficult to create a separate mock object; I already tried to do that and gave up. :) Trustin On Dec 27, 2007 10:48 AM, Trustin Lee [EMAIL PROTECTED] wrote: Hi Jeff and Emmanuel, You can add some hook for IoSession.write() by inserting an IoFilter, so I am not sure about removing the final modifier from write(). Overriding getScheduledMessages and getScheduledBytes makes sense though because they will always be 0 because messageSent event will be always fired immediately. WDYT? Trustin On Dec 27, 2007 10:21 AM, Emmanuel Lecharny [EMAIL PROTECTED] wrote: Jeff Genender wrote: What do *you* think? ;-) Beside the assert, which was a side effect of my quick glance at the method you want to override, I think that dooming the final is sane. If someone needs to protect this method, then the best solution would be to define a super class with final methods calling the AbstractIoSession non final methods. Another solution (puke) would be to remove the public/protected/private qualifier, and to add a MockIoSession in the same package. Not very elegant, but does the job too... There are always tradeoff when you want to thoroughly unit-test some code. When it comes to an API, it seems impossible to avoid those tradeoff, IMHO. Any other opinion ? Trustin ? Julien ? Jeff Emmanuel Lecharny wrote: Jeff Genender wrote: Hey guys, I was hoping to see if we could discuss some of the final qualifiers on some of the methods in the AbstractIOSession. The reason I ask is it would be cool to be able to override some of the methods such as: public WriteFuture write(Object message) public final int getScheduledWriteMessages() To be able to write MockObjects for unit tests of code. Any thoughts on making these protected instead of final? Thanks, Jeff Hi, makes sense to me... Or we may have a AbstractMockIoSession implementing the IoSession interface, for unit tests ? While looking at the abstract class, I found some methods with this kind of code : public final WriteFuture write(Object message, SocketAddress remoteAddress) { if (message == null) { throw new NullPointerException(message); } ... Wouldn't be a perfect case for an assert ? Like : public final WriteFuture write(Object message, SocketAddress remoteAddress) { assert message != null : Null messages are not allowed; ... wdyt ? -- -- cordialement, regards, Emmanuel Lécharny www.iktek.com directory.apache.org -- what we call human nature is actually human habit -- http://gleamynode.net/ -- PGP Key ID: 0x0255ECA6