[jboss-dev-forums] [Design of JBossCache] - Re: Last chance for any changes to the 2.0.0 API

[manik.surtani@jboss.com]

Good comments.

"genman" wrote : 
  | 1. putIfNull -> putIfAbsent, See http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/ConcurrentMap.html . People might get confused with "null" versus absent.
  | 

Agreed.

"genman" wrote : 
  | 2. Remove the print* methods in NodeSPI. Or at least document them. Consider creating a separate util class to print a node.
  | 

My issue with a separate util class is that it will mean maintaining yet another public interface/class.  

+1 to documenting them better.

"genman" wrote : 
  | 3. removeChildDirect(Fqn) and removeChildDirect(Object) is a little confusing, since Fqn is a subtype of Object. I would probably suggest just having one or the other.
  | 

It does look confusing, but for the sake of convenience - and a lot of unnecessary Fqn creation - it makes sense having the 'Object' version.  Helps avoid a lot of unnecessary stuff like 
node.getChildDirect(new Fqn(childName))
which in turn simply does
if (fqn.size() == 1) return children.get(fqn.getLastElement());

Justifying the Fqn version, it cleans up unnecessary and repeated looping in interceptors and CacheImpl searching for a child node several nodes deep.

"genman" wrote : 
  | 4. Some of the NodeSPI data methods appear to be just convenience methods and aren't strictly necessary given that getDataDirect() returns an externally modifiable map. For instance, clearDataDirect() really just is the same as NodeSPI.getDataDirect().clear(). So, it would be nice to understand the reason for these extra methods.

This is a good point.  Will revisit thise.  Their Node interface counterparts that went up the interceptor stack made sense, but the 'direct' versions do not.

View the original post : http://www.jboss.com/index.html?module=bb&op=viewtopic&p=3999715#3999715

Reply to the post : http://www.jboss.com/index.html?module=bb&op=posting&mode=reply&p=3999715