admin@glassfish.java.net

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

From: Lloyd L Chambers <Lloyd.Chambers_at_Sun.COM>
Date: Fri, 04 May 2007 10:50:42 -0700

Yes. :) It won't compile otherwise.

My understanding is that annotations are classnames, and therefore
are case-sensitive.

On May 4, 2007, at 10:12 AM, Byron Nevins wrote:

> 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
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>