jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jukka Zitting <jukka.zitt...@gmail.com>
Subject Re: [jr3] Tree model
Date Wed, 29 Feb 2012 06:31:02 GMT

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.

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:

* 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.

* 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.


Jukka Zitting

View raw message