views:

159

answers:

2

edit

Clafification: The intention is not to remove the node from the original list. But to create an identical node (data and children wise) to the original and insert that into the new list. In other words, a "move" does not imply a "remove" from the original.

endedit

The requirements:

  • Each Node in the list must contain a reference to its previous sibling
  • Each Node in the list must contain a reference to its next sibling
  • Each Node may have a list of child nodes
  • Each child Node must have a reference to its parent node

Basically what we have is a tree structure of arbitrary depth and length. Something like:

-root(NULL)
--Node1
----ChildNode1
------ChildOfChild
--------AnotherChild
----ChildNode2
--Node2
----ChildNode1
------ChildOfChild
----ChildNode2
------ChildOfChild
--Node3
----ChildNode1
----ChildNode2

Given any individual node, you need to be able to either traverse its siblings. the children, or up the tree to the root node.

A Node ends up looking something like this:

class Node
{
  Node* previoius;
  Node* next;
  Node* child;
  Node* parent;
}

I have a container class that stores these and provides STL iterators. It performs your typical linked list accessors. So insertAfter looks like:

void insertAfter(Node* after, Node* newNode)
{
  Node* next = after->next;
  after->next = newNode;
  newNode->previous = after;
  next->previous = newNode;
  newNode->next = next;
  newNode->parent = after->parent;
}

That's the setup, now for the question. How would one move a node (and its children etc) to another list without leaving the previous list dangling?

For example, if Node* myNode exists in ListOne and I want to append it to listTwo.

Using pointers, listOne is left with a hole in its list since the next and previous pointers are changed. One solution is pass by value of the appended Node. So our insertAfter method would become:

void insertAfter(Node* after, Node newNode);

This seems like an awkward syntax. Another option is doing the copying internally, so you'd have:

void insertAfter(Node* after, const Node* newNode)
{
  Node *new_node = new Node(*newNode);
  Node* next = after->next;
  after->next = new_node;
  new_node->previous = after;
  next->previous = new_node;
  new_node->next = next;
  new_node->parent = after->parent;
}

Finally, you might create a moveNode method for moving and prevent raw insertion or appending of a node that already has been assigned siblings and parents.

// default pointer value is 0 in constructor and a operator bool(..) 
// is defined for the Node

bool isInList(const Node* node) const
{
  return (node->previous || node->next || node->parent);
}
// then in insertAfter and friends
if(isInList(newNode)
  // throw some error and bail

I thought I'd toss this out there and see what folks came up with.

+3  A: 

Let's call the node that we are going to remove current. Only the nodes pointed to by current, namely current->previous, current->next, current->child, and current->parent refer to the node current. For example, current->previous->next == current if the current node's previous node is non-null. So, we can easily excise the current node from the list like so:

Vertical Links
A node keeps its children, but it needs to be excised from its parent. This is done like so:

Let parent = current->parent
if parent is non-null:
    if parent->child == current:
         parent->child = current->next
current->parent = null

Horizontal Links
The following code unlinks the current node in the horizontal (next/previous) direction:

Let prev = current->previous
Let next = current->next
if prev is non-null:
    prev->next = next
if next is non-null:
    next->previous = prev
current->previous = null
current->next = null

Granted, this is still a bit messy, but if you break the linked list functionality into small functions (which it looks like you are already doing) and use good comments, then it's really not that bad, I don't think.

Michael Aaron Safyan
Thank you for your response. I may not have made it clear, but the intention is not to remove it from the other list. But to create an identical node (data and children wise) to the original and insert that into the new list. In other words, a "move" does not imply a "remove" from the original.
jkyle
@krunk, so you mean a "copy", not a "move". So, why not create a "treeCopy" function that takes a node and returns a copy with its children copied recursively, then insert that result of the tree copy into the location where you want to insert it.
Michael Aaron Safyan
A: 

First, I agree that, if you're copying the node and not actually removing it from the original tree, the operation should be called a copy, not a move!

Second, I recommend you separate the operation that actually does the copying from the operation that does insertion. This will make it much more flexible, if say for example you need to insert nodes from other sources into your destination, or you want to copy the node for other purposes.

Third, you didn't specify whether Node is polymorphic. If it is, I would implement a method like the following to do the copy:

virtual Node* clone();

You have a couple of design decisions to make here:

  • Do you want to preserve next, prev, and parent when you do a clone? I think it makes sense to zero them out.
  • Do you want to do a deep copy of the children as well when you do a clone? Depending on the use case, you may want to do one or the other, or allow for both.

Fourth, you assume that insertion should fail if the parent/prev/next pointers are already set. You can do this, but you may benefit down the road from a more powerful insertion operation that simply removes nodes from their existing tree in this case. Either will behave consistently.

Fifth, you can't be passing const Nodes around if you're going to be fixing up that node's pointers.

Let's assume for the moment that you decide to zero out the linked list pointers but decide to deep copy the children. Then your code might look like:

class Node {
public:
    Node() : prev(NULL), next(NULL), parent(NULL), child(NULL) { }
    virtual Node* clone() {
        Node* newN = new Node();
        newN->cloneChildren(*this);
        return newN;
    }
    Node* lastChild() const { /* left as exercise */ }
    void insert(Node* node_) { insertAfter(node_, lastChild()); }
    void insertAfter(Node* node_, Node* prevSibling_) {
        ASSERT(node_);
        if (! prevSibling_) { // assume we want to push to front of child list
           prevSibling_ = child; // will be NULL if we have no children
        }
        ASSERT(! prevSibling_ || prevSibling_->parent == this);
        if (node_->parent) {
            // assume you want to move the child in this case
            node_->parent->remove(node_);
        }
        node_->parent = this;
        node_->prev = prevSibling_;
        if (prevSibling_) {
            node_->next = prevSibling_->next;
            prevSibling_->next = node_;
        } else {
            /* the new child is the only child - left as exercise */
        }
    }
    void remove(Node* child_) { /* left as exercise */ }
protected:
    virtual void cloneChildren(const Node& rhs) { /* left as exercise */ }
};

And the code that does the copying from one tree to another simply looks something like:

Node* myNode = ...
Node* someChildInListTwo = findPlaceToInsert(listTwo);
listTwo->insertAfter(myNode->clone(), someChildInListTwo);

In this case we have turned your one somewhat tangled operation into a set of discrete operations that you can use in a wide variety of cases:

  • a clone() operation that allows you to copy arbitrary nodes, whether or not they've been added to trees yet
  • an insert() operation that will automatically do a move for you if you want, and correctly handles adding to the front of the child list
  • a remove() operation that fell out naturally as a byproduct of how we wanted insertion to behave, but which can be used on its own
  • a simple mechanism to copy+insert the node if you don't want to remove the node from the original tree
Owen S.
This is pretty much along the lines I was going with it. I'll be doing shallow copies and possibly extending that to a lazy copy if the need presents itself.
jkyle