On Feb 13, 2009, at 12:52 PM, Otto Hirr wrote:
IMO regardless, even if it makes the recipe more complex, a
recipe should be _correct_ which this is not.
I disagree with that actually. The recipe is correct, it just makes
some assumptions. If this were real world production code it would not
make these assumptions, but since it is meant to be simple
illustrative code primarily for education purposes I think it is okay
to make these assumptions.
And really if we take a look, the only assumption we are making is
that the user of this code is not trying to insert a tree node that is
already connected to some other part of the tree. I would say that
this is user error because this would likely lead cycles (assuming
both nodes were connected to the same tree) and then you wouldn't have
a tree, you would have a graph.
Furthermore the test script never tests this setting condition.
You are correct, it does not. However, I just committed it so it will
be in the next release. I also updated the code as well to be a little
more defensive and to just not accept nodes which are already
connected elsewhere. Here is what it is now:
before 'right', 'left' => sub {
if (defined $tree) {
confess "You cannot insert a tree which already has a parent"
if $tree->has_parent;
$tree->parent($self);
}
};
This puts a little more burden on the user to make sure they have all
their nodes managed correctly, but as I said above this code is for
instructional purposes so I think that is okay.
- Stevan
