Hi, On Mon, May 14, 2012 at 6:43 PM, Michael Dürig <[email protected]> wrote: > We had similar discussions before [1] and it keeps popping up all over > again: The NodeStore interface and friends all operate on NodeState and as I > said before, I don't think there is a reasonable way to implement this.
See my last two commits to .oak.plugins.memory for a rough in-memory outline of how I envisioned the NodeStore mechanism working. > Having NodeState all the places is too general. Have a look at my commits > related to OAK-100. The way things are implemented there is by lying: > NodeStore.getBuilder() and NodeStore.setRoot() only accept very specific > instances of NodeStates as their arguments and throw exceptions on all other > instances. I think we've been heading in a bit different directions with this part of the codebase. A good time to sync up... :-) The way I see it, the NodeStateBuilder (NSB) is a pretty simple construct. All it ever needs to keep track is the set of changes to be applied to a single base NodeState. The context, path and parent information in the current KernelNodeState is in my view not needed. And even further, the implementation of NSB.getNodeState() by calling context.getNodeState(getPath()) goes against the (in my view useful) concept of separate NSB instances being completely independent, each being responsible only for tracking the changes made against that specific instance. I see where you're going by making the NSB context keep track of the full set of transient changes. I had considered that to rather be a concern of the TreeImpl class. Instead of a shared root builder, each TreeImpl would keep a separate NodeStateBuilder instance for tracking the set of transient changes that apply to just that TreeImpl instance. And I'd use NodeStore.compare() for constructing the JSON diff to be passed down to the MicroKernel. Since the NodeStore implementation knows the implementation class of most of the involved NodeState instances, it can even construct things like move and copy instructions in the NodeStateDiff handler. > I think this is wrong and we should use more exact types for > NodeStore (and related) interfaces instead of throwing exceptions at > runtime. We can look at reorganizing the method calls so that fewer class casts are needed. But if the contracts are revised as described above I don't think there'll be too many places where casting to implementation classes is needed, and most of them will have reasonable (though potentially much slower) fallbacks to generic types. BR, Jukka Zitting
