Hi,
On 29.2.12 6:31, Jukka Zitting wrote:
Hi,
On Wed, Feb 29, 2012 at 1:54 AM, Michael Dürig<[email protected]> 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.
Michael
