[jira] [Commented] (COLLECTIONS-148) [collections] Change to HashEntry inner class of AbstractHashedMap

["Thomas Neidhart (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/COLLECTIONS-148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13642126#comment-13642126 ] 

Thomas Neidhart commented on COLLECTIONS-148:
---------------------------------------------

Postpone to a later version.
                
> [collections] Change to HashEntry inner class of AbstractHashedMap
> ------------------------------------------------------------------
>
>                 Key: COLLECTIONS-148
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-148
>             Project: Commons Collections
>          Issue Type: Improvement
>         Environment: Operating System: other
> Platform: All
>            Reporter: Henry Story
>            Priority: Minor
>             Fix For: 4.x
>
>         Attachments: ASF.LICENSE.NOT.GRANTED--collections-altered.jpg, ASF.LICENSE.NOT.GRANTED--collections.jpg, ASF.LICENSE.NOT.GRANTED--hashed.tar, ASF.LICENSE.NOT.GRANTED--patch.txt
>
>
> The way the AbstractHashedMap class is currently written it is not as extensible as it could be.
> The HashEntry static inner class is abstract. By slightly refactoring it to an inner interface one can get a 
> lot more mileage out of it.
> This change has minimal impact on four other classes from the map package.
> Two extra classes which I will submit as a seperate patch (should I do this before this one is accepted?)
> that use this inner interface are a HashedSet and a WeakHashedSet. Both of these classes are adapters 
> of a subclass of the changed AbstractHashedMap.
> The HashEntry interface essentially hides the representation of the key and value objects. In a HashSet 
> the key is the value, so this avoids the duplication - not of objects, but of references. More importantly 
> it also allows a HashEntry to extend a WeakReference Object, which cannot be done as the code 
> presently stands.
> Here is the new interface:
>     protected static interface HashEntry  extends Map.Entry, KeyValue {
>          public HashEntry getNext();
>          public void setNext(HashEntry next);
>          public int getHashCode();
>          public void setHashCode(int hashCode);
>         /**
>           * @param key raw key (ie, no interpretation for special cases like NULL
>          */
>         public void setRawKey(Object key);
>         /**
>          *
>          * @return the raw key
>          */
>         public Object getRawKey();
>     }
> This allows the implementation to decide how they can refer to the key and the values. Essentially we 
> remove all reference in the code to the variables 'key' and 'value' and replace them with the (set/
> get)rawkey (set/get)value methods. The raw key method is necessary as the setKey method is often 
> overridden to do special null substition work.
> I have also created a more interesting NULL object, that can better describe itself. When debugging it is 
> often helpful to know that one is looking at a NULL object.
> Finally I also - and debatably wrongly  - changed the Iterator from  static inner classes to real inner 
> classes. This is how it is meant to work anyway. An inner class just has a reference to its enclosing 
> class. But perhaps there is something here that I have not understood. My other contributions don't 
> hang on this change. They just make the code a little simpler. 
> If the intention is to extract these classes to an external package, then the current solution of having a 
> static inner class makes sense.
> I will attach class diagrams and diffs to this request to clarify the changes.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira