admin@glassfish.java.net

Re: CODE REVIEW: FindBugs: /sun/enterprise/ee/admin/PortInUse.java

From: Byron Nevins <Byron.Nevins_at_Sun.COM>
Date: Fri, 04 May 2007 10:12:27 -0700

It's @Override, not @override

Lloyd L Chambers wrote:
> Done.
>
> On May 3, 2007, at 4:22 PM, Kedar Mhaswade wrote:
>
>> Add @override for hashCode().
>> Add @override for equals(Object o).
>>
>>
>> Lloyd L Chambers wrote:
>>> New diffs. I noticed (because of Bill's earlier comments), that
>>> equals() looks *only* at '_port'. Changed to make hashCode() use
>>> '_port' only.
>>>
>>> Index:
>>> admin-ee/admin/src/java/com/sun/enterprise/ee/admin/PortInUse.java
>>> ===================================================================
>>> RCS file:
>>> /cvs/glassfish/admin-ee/admin/src/java/com/sun/enterprise/ee/admin/PortInUse.java,v
>>>
>>> retrieving revision 1.1.1.1
>>> diff -w -u -r1.1.1.1 PortInUse.java
>>> ---
>>> admin-ee/admin/src/java/com/sun/enterprise/ee/admin/PortInUse.java
>>> 8 Aug 2006 19:48:39 -0000 1.1.1.1
>>> +++
>>> admin-ee/admin/src/java/com/sun/enterprise/ee/admin/PortInUse.java
>>> 3 May 2007 22:38:57 -0000
>>> @@ -33,16 +33,17 @@
>>> *
>>> * In addition a conflicting port may optionally be resolved to a
>>> non-conflicting port.
>>> * If this is the case then
>>> + <b>NOT THREAD SAFE (mutable variables)</b>
>>> */
>>> -public class PortInUse implements Serializable {
>>> +public final class PortInUse implements Serializable {
>>>
>>> public static final int NO_NEW_PORT = -1;
>>>
>>> - private String _propertyName = null;
>>> - private int _port;
>>> - private String _hostName = null;
>>> - private String _serverName = null;
>>> - private String _conflictingPropertyName = null;
>>> + private final String _propertyName;
>>> + private final int _port;
>>> + private final String _hostName;
>>> + private final String _serverName;
>>> + private final String _conflictingPropertyName;
>>> private int _newPort = NO_NEW_PORT;
>>> /**
>>> @@ -150,13 +151,25 @@
>>> return false;
>>> }
>>> }
>>> - /**
>>> - * this overriding method is temporary added to avoid findBugs
>>> warnings
>>> - * it intentionally not based on port number
>>> - * BTW, I could not find any usage of PortInUse.equals so it
>>> maybe unecessary
>>> +
>>> + public int hashCode() {
>>> + return _port;
>>> +
>>> + /* probably ought to be as follows, but equals() was originally
>>> + written to use *only* the port.
>>> +
>>> + private static int
>>> + hashIt( final Object o ) {
>>> + return o == null ? 0 : o.hashCode();
>>> + }
>>> +
>>> +
>>> + return _port ^ _newPort ^
>>> + hashIt( _propertyName ) ^
>>> + hashIt( _hostName ) ^
>>> + hashIt( _serverName ) ^
>>> + hashIt( _conflictingPropertyName );
>>> */
>>> - public int HashCode()
>>> - {
>>> - return ((Object)this).hashCode();
>>> }
>>> +
>>> }
>>>
>>>
>>>
>>> On May 3, 2007, at 12:11 PM, Lloyd L Chambers wrote:
>>>
>>>> TIMEOUT: 17:00 PST May 3
>>>> https://glassfish.dev.java.net/issues/show_bug.cgi?id=2965
>>>>
>>>> dpatil: com/sun/enterprise/ee/admin/PortInUse.java:147:150 HE:
>>>> com.sun.enterprise.ee.admin.PortInUse defines equals and uses
>>>> Object.hashCode() (H) dpatil:
>>>> com/sun/enterprise/ee/admin/PortInUse.java:160:160 Nm: VERY
>>>> confusing to have methods
>>>> com.sun.enterprise.ee.admin.PortInUse.HashCode() and
>>>> java.lang.Object.hashCode() (H)
>>>>
>>>> Index:
>>>> admin-ee/admin/src/java/com/sun/enterprise/ee/admin/PortInUse.java
>>>> ===================================================================
>>>> RCS file:
>>>> /cvs/glassfish/admin-ee/admin/src/java/com/sun/enterprise/ee/admin/PortInUse.java,v
>>>>
>>>> retrieving revision 1.1.1.1
>>>> diff -w -u -r1.1.1.1 PortInUse.java
>>>> ---
>>>> admin-ee/admin/src/java/com/sun/enterprise/ee/admin/PortInUse.java
>>>> 8 Aug 2006 19:48:39 -0000 1.1.1.1
>>>> +++
>>>> admin-ee/admin/src/java/com/sun/enterprise/ee/admin/PortInUse.java
>>>> 3 May 2007 18:54:12 -0000
>>>> @@ -33,16 +33,17 @@
>>>> *
>>>> * In addition a conflicting port may optionally be resolved to a
>>>> non-conflicting port.
>>>> * If this is the case then
>>>> + <b>NOT THREAD SAFE (mutable variables)</b>
>>>> */
>>>> -public class PortInUse implements Serializable {
>>>> +public final class PortInUse implements Serializable {
>>>>
>>>> public static final int NO_NEW_PORT = -1;
>>>>
>>>> - private String _propertyName = null;
>>>> - private int _port;
>>>> - private String _hostName = null;
>>>> - private String _serverName = null;
>>>> - private String _conflictingPropertyName = null;
>>>> + private final String _propertyName;
>>>> + private final int _port;
>>>> + private final String _hostName;
>>>> + private final String _serverName;
>>>> + private final String _conflictingPropertyName;
>>>> private int _newPort = NO_NEW_PORT;
>>>> /**
>>>> @@ -150,13 +151,18 @@
>>>> return false;
>>>> }
>>>> }
>>>> - /**
>>>> - * this overriding method is temporary added to avoid findBugs
>>>> warnings
>>>> - * it intentionally not based on port number
>>>> - * BTW, I could not find any usage of PortInUse.equals so it
>>>> maybe unecessary
>>>> - */
>>>> - public int HashCode()
>>>> - {
>>>> - return ((Object)this).hashCode();
>>>> +
>>>> + private static int
>>>> + hashIt( final Object o ) {
>>>> + return o == null ? 0 : o.hashCode();
>>>> }
>>>> +
>>>> + public int hashCode() {
>>>> + return _port ^ _newPort ^
>>>> + hashIt( _propertyName ) ^
>>>> + hashIt( _hostName ) ^
>>>> + hashIt( _serverName ) ^
>>>> + hashIt( _conflictingPropertyName );
>>>> + }
>>>> +
>>>> }
>>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>