jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Dürig <mic...@gmail.com>
Subject Re: [jr3] Tree model
Date Wed, 29 Feb 2012 11:00:32 GMT


On 29.2.12 6:31, Jukka Zitting wrote:
> Hi,
> On Wed, Feb 29, 2012 at 1:54 AM, Michael Dürig<mduerig@apache.org>  wrote:
>> See https://gist.github.com/1934912 for what I meant about factoring
>> mutability out for transient modifications into a decorator on top of a
>> completely immutable (i.e. persistent) Tree.
> I see where you're going, but with "MutableTree implements Tree" (i.e.
> MutableTree ISA Tree) we still fail to make a categorical statement
> that "all Trees are immutable". In other words all you gain is a
> marker type for making it easier to track whether a given Tree
> instance is supposed to be mutable or not. Using the Factory (or
> Builder) pattern as you originally proposed would IMHO be better from
> a conceptual perspective, but that'll make it harder for something
> like a transient space to read content that's still being modified.
> And whichever way we decide, it should be an interface instead of a
> concrete class.

Sorry I wasn't too clear here about my intent. The MutableTree class is 
meant as an example of an internal implementation (e.g. in the transient 
space). That is, I would remove mention of mutability from the Tree 
interface completely such that the contract is "a Tree is immutable". 
Internal implementations like MutableTree are free to add mutator 
methods to modify a tree as long as they ensure the mutability is 
local/private and doesn't leak to the outside. For MutableTree this is 
very much the case: the original tree instance is not touched at all and 
as you only use mutator methods before you pass the MutableTree to other 
APIs expecting a Tree, there is no problem here neither.

> I also notice you replaced "extends Map" with a set of custom method
> signatures in Tree. As discussed I'm fine with this change, but there
> are now some subtle issues that should be addressed:

Yes, this is by no means finished though and I'm not really sure what's 
best here. I find extending from Map too heavy since it forces every 
implementation to either re-implement the Map methods or to extend from 
a common base class which provides these methods. Furthermore Map 
emphasises mutability which I wouldn't. OTOH implementing the Map 
interface is surely of great value to consumers. Overall I'd prefer a 
utility method (or class) along the lines of:

public static Map<String, Tree> toMap(Tree tree) {...}

> * By dropping signatures like entrySet() and values(), you force all
> subtree access to happen by string lookups. That might have a negative
> performance impact on some access patterns (for example a garbage
> collector that just needs to traverse the full tree).
> * Returning an Iterator instead of an Iterable from getChildNames()
> makes it unusable in foreach loops.

Right, these should return Iterable then.

> * The asLeaf() and accept() methods are now somewhat overloaded. I'd
> only put one of them in the interface, and implement the other as a
> utility method in a separate class.

True. Since there is no really good way to deconstruct algebraic data 
types in Java I'm always a bit torn between the one and the other: 
asLeaf() is fine as long as you only care about this one instance and a 
full blown visitor is way too heavy weight. OTOH if you need to 
deconstruct a whole tree and accumulate some state a visitor is a much 
better fit.


View raw message