Re: [net][patch] Added more robust SocketClient setup/cleanu

["Daniel F. Savarese" <df...@savarese.org>]

In message <3F...@users.sourceforge.net>, Jason Mathews writes:
>Added more robust setup and cleanup to UDP and SocketClient base classes 
>including Time and Daytime TCP & UDP clients to for example check if not 
>connected and either throw an IOexception or force a open connection 
>rather than throw a NullpointerException.

As far as I can tell, these changes are not needed and I recommend
against applying them.  One should not call close() if one never calls
open() first.  We can't check for every improper use of an API.
Perhaps I have misunderstood the intent, but I just don't see what
these changes buy anyone.  I also don't see any point to the changes
to Daytime/TimeUDPlient.  The API convention isn't that you call getTime()
and for getTime() to check to see if a socket has been opened and then to
open one.  The API is that you call open(), then call getTime().

The only change I might concede has some merit is the !isConnected() check
in Daytime/TimeTCPClient, but, in principle I'm opposed to it because I
don't believe these classes should check for every possible state violation.
If a programmer chooses to ignore the API and invoke a protocol method
before establishing a connection with open(), then he deserves the possibly
cryptic NullPointerException he gets.  It's not like these classes
asynchronously close connections and set member variables to null.  The 
state of the classes is fully under the control of the programmer.

It's late and I may be missing something, so please enlighten me if I am.

daniel

Re: [net][patch] Added more robust SocketClient setup/cleanu

[Jason Mathews <gj...@users.sourceforge.net>]

Daniel F. Savarese wrote:

>In message <3F...@users.sourceforge.net>, Jason Mathews writes:
>  
>
>>Added more robust setup and cleanup to UDP and SocketClient base classes 
>>including Time and Daytime TCP & UDP clients to for example check if not 
>>connected and either throw an IOexception or force a open connection 
>>rather than throw a NullpointerException.
>>    
>>
>
>As far as I can tell, these changes are not needed and I recommend
>against applying them.  One should not call close() if one never calls
>open() first.  We can't check for every improper use of an API.
>Perhaps I have misunderstood the intent, but I just don't see what
>these changes buy anyone.  I also don't see any point to the changes
>to Daytime/TimeUDPlient.  The API convention isn't that you call getTime()
>and for getTime() to check to see if a socket has been opened and then to
>open one.  The API is that you call open(), then call getTime().
>  
>
I tend to agree that force calling open() in getTime/getDate()  might be 
assuming too much on part of the user of the class (not following API, 
etc.) so again throwing a IOException("not connected) could be better 
there too.

But checking if a socket is null in the close() for example is good 
practice. Often a programmer will close the Socket, JDBC connection, etc 
in a finally block whether or not is was or still is connected. The 
Socket or SocketClient class in this case knows whether it is open and 
thowing a NullpointerException does not seem right to me.

Agree that the classes should not check for every possible state 
violation but checking the most common ones makes the class more robust. 
Should not be overkill to add some extra checks -- this follows the 
coding standards of the java.net and java.sql classes (URLConnection, 
Socket, etc.)

--jason

>The only change I might concede has some merit is the !isConnected() check
>in Daytime/TimeTCPClient, but, in principle I'm opposed to it because I
>don't believe these classes should check for every possible state violation.
>If a programmer chooses to ignore the API and invoke a protocol method
>before establishing a connection with open(), then he deserves the possibly
>cryptic NullPointerException he gets.  It's not like these classes
>asynchronously close connections and set member variables to null.  The 
>state of the classes is fully under the control of the programmer.
>
>It's late and I may be missing something, so please enlighten me if I am.
>
>daniel
>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>  
>
-- 
Jason Mathews <gj...@users.sourceforge.net>
The MITRE Corporation <http://www.mitre.org/>
Bedford, MA 01730-1407