Good comments.
"genman" wrote :
| 1. putIfNull -> putIfAbsent, See
http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/ConcurrentMa... . 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#...
Reply to the post :
http://www.jboss.com/index.html?module=bb&op=posting&mode=reply&a...