commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael A. Smith" <...@apache.org>
Subject Re: PATCH : org.apache.commons.collections.SoftRefHashMap
Date Tue, 21 May 2002 14:31:52 GMT
On Tue, 21 May 2002, christopher marshall wrote:
> It doesn't seem as if anyone has submitted this patch into the CVS 
> repository for testing... Is this any way to encourage new participants?

we're all volunteers here.  I can only speak for myself, but I've been 
extremely busy and haven't had much time for anything lately.  However, 
here's a quick review of your mail:

> >Please let me know if this is OK as I haven't submitted code before.

patches are preferred.  See the bottom section of this document: 
http://jakarta.apache.org/site/source.html

see also:
http://jakarta.apache.org/site/getinvolved.html

> >Patches fix the SoftRefHashMap so that the "entrySet()" and "values()"
> >methods are backed by the underlying map and the keys whose referent-values
> >have been cleared will be automatically (and performant-ly) purged upon
> >writes to the map. I have deprecated the "purge()" method and 
> >re-implemented
> >it to call "processQueue()", which is a more performant way of purging the
> >Map as it goes straight to the cleared values without iterating through the
> >whole Map.

Having the entrySet and values collections backed by the map is probably a 
good thing.  As for the purge stuff, I don't know what your motivation is 
here.  "performant" isn't in my trusty on-line dictionaries:
http://www.dictionary.com/search?q=performant
http://www.m-w.com/cgi-bin/dictionary?performant
Are you saying it has better performance?  That's probably good as well.

> >The Map also has the new method "getIfAbsentPut(Object key, ObjectFactory
> >fac)" which takes a new paramter of type
> >org.apache.common.collections.ObjectFactory (an Interface). It aids the use
> >of this map as a cache (see below).

In my opinion, this is not appropriate to add to the SoftRefHashMap.  If
you want such functionality, add it in a subclass

> >I feel that the idea to extend the class with "createReference()"
> >overimplemented is a bad one, and have removed the functionality. Among
> >other things, it is difficult to see how this would be implemented here
> >without unnecessary complications - considering the functionality is
> >questionable (to extend SoftRefHashMap to actually be a WeakRefHashMap?) I
> >think that this is justified.

well, I don't.  You're removing functionality and if you just removed the 
method, you're breaking backwards compatibility.  what unnecessary 
complications are you talking about?  Just a possible misleading super 
class?  How else is this functionality questionable? 

> >The idea of Object factory is so that the Map can be used effectively as a
> >Cache; suppose that a User wishes to store (expensively retrieved) database
> >entries in the Map which must always be retrieved (whether the referent
> >value has since been GC-ed or not). The user code looks like
> >
> >//
> >public UserObject apiGetUser(final String userId) {
> >  return (UserObject) map.getIfAbsentPut(userId, new ObjectFactory() {
> >     public Object create() {
> >         //expensive database access to return the
> >         //required Object
> >         Object user = expensiveDBAccess( userId );
> >         return user;
> >     }
> >  });
> >}
> >//
> >
> >The ObjectFactory.create() will only be invoked (and the expensive 
> >operation
> >executed) should the value have been cleared from the Map (ie. GC-d). The
> >factory method will be invoked and the Object stored in the map for the 
> >key.

this isn't necessary.  The SoftRefHashMap can do this without 
modification. 

  UserObject obj = (UserObject)map.get(userId);
  if(obj == null) {
    obj = expensiveDBAccess(userId);
    map.put(userId, obj);
  }

The only thing you might gain from the object factory thing is to remove 
possible code duplication if you're doing these map.get things in various 
places in your code.  But in that case, I'd recommend you create a 
separate object that stores your user information and handles this under 
the covers.  For example:

  public class UserCache {
    private SoftRefHashMap map = new SoftRefHashMap();
   
    public UserObject getUserObject(String userId) {
      UserObject obj = (UserObject)map.get(userId);
      if(obj == null) {
        obj = expensiveDBAccess(userId);
        map.put(userId, obj);
      }
    }
  }

or as a direct subclass:

  public class UserCache extends SoftRefHashMap {
    public Object get(Object key) {
      Object o = super.get(key);
      if(o == null) {
        o = expensiceDBAccess(key);
        map.put(key, o);
      }
    }
  }



regards,
michael


--
To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>


Mime
View raw message