Last minute 2.0 API suggestions

[genman]

1. Have Node.removeChild() return true if the node was removed
2. Add a Node.size() or Node.dataSize() to count basically getData().size(). The problem (in terms of efficiency) is that getData() makes a copy.
3. Node.putIfAbsent() should be consistent with ConcurrentMap, and return Object. Consider adding other methods of ConcurrentMap.
4. Change Node.put to Node.putAll to be consistent with Map.putAll().

I'm working on a Node -> Map adaptor, sort of a poor man's PojoCache, and find these basic methods missing.

[manik]

"genman" wrote:

1. Have Node.removeChild() return true if the node was removed

Need to think about how big an impact this will have on the interceptor chain since this calls remove(Fqn).

"genman" wrote:

2. Add a Node.size() or Node.dataSize() to count basically getData().size(). The problem (in terms of efficiency) is that getData() makes a copy.
3. Node.putIfAbsent() should be consistent with ConcurrentMap, and return Object. Consider adding other methods of ConcurrentMap.
4. Change Node.put to Node.putAll to be consistent with Map.putAll().

Makes sense. I've added the APIs and basic impl's, which we can revise/refactor over time.

"genman" wrote:

I'm working on a Node -> Map adaptor, sort of a poor man's PojoCache, and find these basic methods missing.

An interesting idea; I'd like to see a separate thread on this though so we can discuss

[manik]

"genman" wrote:

1. Have Node.removeChild() return true if the node was removed

Need to think about how big an impact this will have on the interceptor chain since this calls remove(Fqn).

This is actually not a good idea - the whole concept of Nodes are Nodes that are attached to a tree. If a node has been removed, a reference to it will not be of much use at all.

[genman]

1. It would be nice if it returned a boolean not a Node
2. Actually, I'm not sure "size" is a good idea. It still has to hit the interceptor chain to return the correct value, and for this I assume requires some non-trivial thought.

I'm creating a thread for the Cache->Map thing

[manik]

1. It would be nice if it returned a boolean not a Node

is this specifically useful though?

2. Actually, I'm not sure "size" is a good idea. It still has to hit the interceptor chain to return the correct value, and for this I assume requires some non-trivial thought.

Well, retrieving the data or keys would also involve the interceptor stack and this is just a convenience atop that. But more than that, I'm doing it to get it in to the API. We can optimise and improve on this later, but at worst it will have the same overhead as doing getKeys().size()

[genman]

The Collection interface has a "boolean remove(Object k)", and it's nice (when writing adaptors, see that other thread) to see if the remove operation actually worked.

One good thing about the Collection interfaces is that they always return information. This suggests that all Node methods should return something. Some thoughts:

NodeSPI -
void setVersion(DataVersion version); - Return replaced version?
void removeChildrenDirect(); - Return count of children removed?
void removeChildDirect(Fqn fqn); - boolean true if removed?
void removeChildDirect(Object childName); - ""
Cache -
void removeNode(Fqn fqn); - boolean true if removed
void evict(Fqn fqn, boolean recursive); - ""

I also wonder if Cache.getVersion() shouldn't return a Version (not String) object instead? Version might implement a "before" method as well. It'd be nice for people writing something like this:

Cache c;
if (c.getVersion().before(Version.XYZ))
throw new Exception("Cache not >= Version XYZ");

I'm not sure how often this use case comes up, though.

[manik]

I've accepted your patch about returning booleans for remove()'s.

I'd rather leave out the suggestion re: a Version object for cache version, until a proper use case comes up. Over-engineering, IMO. :-)