We need an explicit test case, even if it is just a copy/subclass. Stephen > from: Neil O'Toole <[EMAIL PROTECTED]> > date: Wed, 24 Sep 2003 09:20:01 > to: [EMAIL PROTECTED], [EMAIL PROTECTED] > subject: Re: [collections] Map.Entry and KeyValue > > I believe that the test case for DefaultMapEntry will test by extension > test KeyValuePair??.. though we can simply subclass the test case also > - should we do this? > > N. > > --- Stephen Colebourne <[EMAIL PROTECTED]> wrote: > > I'm happy to add it....with a test case :-) > > Stephen > > > > ----- Original Message ----- > > From: "Neil O'Toole" <[EMAIL PROTECTED]> > > > > My suggested solution is to add a class "KeyValue.java" > > containing > > > > the code from DefaultMapEntry, but not implementing Map.Entry. > > > > DefaultMapEntry can then subclass KeyValue and implement > > Map.Entry, > > > > and be declared final. > > > > > > Suggested solution attached (KeyValuePair.java and patch for > > > DefaultMapEntry.java). Note that: > > > > > > - The superflous null check is removed from DefaultMapEntry#equals > > > (instanceof checks for null). > > > - The DefaultMapEntry #equals and #hashCode methods are declared > > final. > > > The 'correct' approach is probably that the whole class should be > > > declared final - thoughts on this??. > > > - KeyValuePair has a toString() method. > > > > > > Unless there are objections, anyone want to add this to CVS? > > > > > > Neil > > > > > > > > > --- Neil O'Toole <[EMAIL PROTECTED]> wrote: > > > > > > ** I had a swift look through [collections] and [lang], and > > > > didn't > > > > > see > > > > > > a simple KeyValuePair implementation... Have I missed this > > > > > somewhere? > > > > > > > > > > DefaultMapEntry IIRC is the simple Map.Entry impl. > > > > > Stephen > > > > > > > > > > > > > Although one (including my good self!) might initially think that > > a > > > > key-value class and a Map.Entry class are interchangeable, this > > is > > > > not > > > > in fact the case. DefaultMapEntry is not suitable for use as a > > > > general > > > > purpose KeyValue class, as it is effectively non-extendable (and > > > > there > > > > are many cases where it is useful to extend a KeyValue class). In > > > > fact, > > > > DefaultMapEntry (or at the very least its #equals and #hashcode > > > > methods) should be declared final so that users won't make the > > > > mistake > > > > of subclassing it. > > > > > > > > DefaultMapEntry is not extendable because the #equals method is > > > > contractually specified by Map.Entry to only evalute the members > > of > > > > Map.Entry. A non-trivial subclass of DefaultMapEntry will add > > members > > > > that will be used in #equals comparision. To make use of these > > > > members > > > > in #equals will of course violate the Map.Entry#equals contract. > > > > > > > > This non-extensibility property applies to any class that > > inherits or > > > > implements an #equals method with a similar closed #equals > > contract, > > > > and any such class should be declared final to prevent accidental > > > > subclassing (or again, at the very least the #hashcode and > > #equals > > > > methods should be declared final, though the user might just > > ignore > > > > those methods, resulting in unfortunate code). > > > > > > > > My suggested solution is to add a class "KeyValue.java" > > containing > > > > the > > > > code from DefaultMapEntry, but not implementing Map.Entry. > > > > DefaultMapEntry can then subclass KeyValue and implement > > Map.Entry, > > > > and > > > > be declared final. > > > > > > > > - Neil > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > To unsubscribe, e-mail: > > [EMAIL PROTECTED] > > > > For additional commands, e-mail: > > [EMAIL PROTECTED] > > > > > > > > > > > > > > ---------------------------------------------------------------------------- > > ---- > > > > > > > /* > > > * $Header: > > > /home/cvspublic/jakarta-commons/collections/src/java/org/apache/commons/coll > > ections/DefaultMapEntry.java,v 1.11 2003/08/31 17:26:43 scolebourne > > Exp $ > > > * > > ==================================================================== > > > * > > > * The Apache Software License, Version 1.1 > > > * > > > * Copyright (c) 2001-2003 The Apache Software Foundation. All > > rights > > > * reserved. > > > * > > > * Redistribution and use in source and binary forms, with or > > without > > > * modification, are permitted provided that the following > > conditions > > > * are met: > > > * > > > * 1. Redistributions of source code must retain the above > > copyright > > > * notice, this list of conditions and the following disclaimer. > > > * > > > * 2. Redistributions in binary form must reproduce the above > > copyright > > > * notice, this list of conditions and the following disclaimer > > in > > > * the documentation and/or other materials provided with the > > > * distribution. > > > * > > > * 3. The end-user documentation included with the redistribution, > > if > > > * any, must include the following acknowledgement: > > > * "This product includes software developed by the > > > * Apache Software Foundation (http://www.apache.org/)." > > > * Alternately, this acknowledgement may appear in the software > > itself, > > > * if and wherever such third-party acknowledgements normally > > appear. > > > * > > > * 4. The names "The Jakarta Project", "Commons", and "Apache > > Software > > > * Foundation" must not be used to endorse or promote products > > derived > > > * from this software without prior written permission. For > > written > > > * permission, please contact [EMAIL PROTECTED] > > > * > > > * 5. Products derived from this software may not be called > > "Apache" > > > * nor may "Apache" appear in their names without prior written > > > * permission of the Apache Software Foundation. > > > * > > > * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED > > > * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED > > WARRANTIES > > > * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE > > > * DISCLAIMED. IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR > > > * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > > > * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > > > * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF > > > * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED > > AND > > > * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT > > LIABILITY, > > > * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY > > OUT > > > * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY > > OF > > > * SUCH DAMAGE. > > > * > > ==================================================================== > > > * > > > * This software consists of voluntary contributions made by many > > > * individuals on behalf of the Apache Software Foundation. For > > more > > > * information on the Apache Software Foundation, please see > > > * <http://www.apache.org/>. > > > * > > > */ > > > package org.apache.commons.collections; > > > > > > > > > > > > /** > > > * A simple key-value pair. > > > * > > > * @since Commons Collections 3.0 > > > * @version $Revision: $ > > > * > > > * @author <a href="mailto:[EMAIL PROTECTED]">James Strachan</a> > > > * @author <a href="mailto:[EMAIL PROTECTED]">Michael A. Smith</a> > > > * @author Neil O'Toole > > > */ > > > public class KeyValuePair{ > > > > > > /** The key */ > > > private Object key; > > > /** The value */ > > > private Object value; > > > > > > /** > > > * Constructs a new <Code>DefaultMapEntry</Code> with a null > > key > > > * and null value. > > > */ > > > public KeyValuePair() { > > > } > > > > > > /** > > > * Constructs a new <Code>DefaultMapEntry</Code> with the given > > > * key and given value. > > > * > > > * @param key the key for the entry, may be null > > > * @param value the value for the entry, may be null > > > */ > > > public KeyValuePair(Object key, Object value) { > > > this.key = key; > > > this.value = value; > > > } > > > > > > > > > /** > > > * Returns true if the compared object is also a > > <code>KeyValuePair</code>, > > > * and its key and value are equal to this object's key and value. > > > */ > > > public boolean equals(Object o) { > > > > > > if ( o == this ) return true; > > > > > > if ( o instanceof KeyValuePair == false ) return false; > > > > > > KeyValuePair e2 = (KeyValuePair) o; > > > return ((getKey() == null ? > > > e2.getKey() == null : > > getKey().equals(e2.getKey())) && > > > (getValue() == null ? > > > e2.getValue() == null : > > getValue().equals(e2.getValue()))); > > > } > > > > > > > > > > > > public int hashCode() { > > > return ( ( getKey() == null ? 0 : getKey().hashCode() ) ^ > > > ( getValue() == null ? 0 : getValue().hashCode() ) > > ); > > > } > > > > > > > > > > > > /** > > > * Returns the key. > > > * > > > * @return the key > > > */ > > > public Object getKey() { > > > return key; > > > } > > > > > > /** > > > * Returns the value. > > > * > > > * @return the value > > > */ > > > public Object getValue() { > > > return value; > > > } > > > > > > > > > > > > /** > > > * Sets the key. > > > * > > > * @param key the new key > > > */ > > > public void setKey(Object key) { > > > this.key = key; > > > } > > > > > > /** > > > * Sets the value. > > > * > > > * @return the old value of the value > > > * @param value the new value > > > */ > > > public Object setValue(Object value) { > > > Object answer = this.value; > > > this.value = value; > > > return answer; > > > } > > > > > > public String toString() > > > { > > > return new > > > StringBuffer().append(getKey()).append('=').append(getValue()).toString(); > > > } > > > } > > > > > > > > > > ---------------------------------------------------------------------------- > > ---- > > > > > > > Index: DefaultMapEntry.java > > > =================================================================== > > > RCS file: > > > /home/cvspublic/jakarta-commons/collections/src/java/org/apache/commons/coll > > ections/DefaultMapEntry.java,v > > > retrieving revision 1.11 > > > diff -u -r1.11 DefaultMapEntry.java > > > --- DefaultMapEntry.java 31 Aug 2003 17:26:43 -0000 1.11 > > > DefaultMapEntry.java 23 Sep 2003 02:00:14 -0000 > > > @@ -67,13 67,10 @@ > > > * > > > * @author <a href="mailto:[EMAIL PROTECTED]">James > > Strachan</a> > > > * @author <a href="mailto:[EMAIL PROTECTED]">Michael A. Smith</a> > > > * @author Neil O'Toole > > > */ > > > -public class DefaultMapEntry implements Map.Entry { > > > public class DefaultMapEntry extends KeyValuePair implements > > Map.Entry { > > > > > > - /** The key */ > > > - private Object key; > > > - /** The value */ > > > - private Object value; > > > > > > /** > > > * Constructs a new <Code>DefaultMapEntry</Code> with a null > > key > > > @@ -90,20 87,18 @@ > > > * @param value the value for the entry, may be null > > > */ > > > public DefaultMapEntry(Object key, Object value) { > > > - this.key = key; > > > - this.value = value; > > > super(key, value); > > > } > > > > > > /** > > > * Implemented per API documentation of > > > * [EMAIL PROTECTED] java.util.Map.Entry#equals(Object)} > > > */ > > > - public boolean equals(Object o) { > > > - if( o == null ) return false; > > > public final boolean equals(Object o) { > > > if( o == this ) return true; > > > > > > - if ( ! (o instanceof Map.Entry ) ) > > > - return false; > > > if ( !(o instanceof Map.Entry ) ) return false; > > > > > > Map.Entry e2 = (Map.Entry)o; > > > return ((getKey() == null ? > > > e2.getKey() == null : > > getKey().equals(e2.getKey())) && > > > @@ -116,57 111,12 @@ > > > * Implemented per API documentation of > > > * [EMAIL PROTECTED] java.util.Map.Entry#hashCode()} > > > */ > > > - public int hashCode() { > > > public final int hashCode() { > > > return ( ( getKey() == null ? 0 : getKey().hashCode() ) ^ > > > ( getValue() == null ? 0 : getValue().hashCode() > > ) ); > > > } > > > > > > > > > > > > - // Map.Entry interface > > > - > > > //------------------------------------------------------------------------- > > > - > > > - /** > > > - * Returns the key. > > > - * > > > - * @return the key > > > - */ > > > - public Object getKey() { > > > - return key; > > > - } > > > - > > > - /** > > > - * Returns the value. > > > - * > > > - * @return the value > > > - */ > > > - public Object getValue() { > > > - return value; > > > - } > > > - > > > - // Properties > > > - > > > //------------------------------------------------------------------------- > > > - > > > - /** > > > - * Sets the key. This method does not modify any map. > > > - * > > > - * @param key the new key > > > - */ > > > - public void setKey(Object key) { > > > - this.key = key; > > > - } > > > - > > > - /** > > > - * Note that this method only sets the local reference inside > > this > > object and > > > - * does not modify the original Map. > > > - * > > > - * @return the old value of the value > > > - * @param value the new value > > > - */ > > > - public Object setValue(Object value) { > > > - Object answer = this.value; > > > - this.value = value; > > > - return answer; > > > - } > > > > > > } > > > > > > > > > > > > > ---------------------------------------------------------------------------- > > ---- > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: [EMAIL PROTECTED] > > > For additional commands, e-mail: > > [EMAIL PROTECTED] > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] >
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
