Use static variables for Node#previous/#next

Instead of calculating the previous/next node on the fly this data is
now set automatically whenever a node is stored in a NodeSet with an
owner. While this introduces some overhead and complexity when adding or
removing nodes from a NodeSet, it greatly reduces the runtime overhead
of calling Node#previous or Node#next.

lib/oga/xml/node.rb

@@ -10,6 +10,12 @@ module Oga
       # @return [Oga::XML::NodeSet]
       attr_reader :node_set
 
+      # @return [Oga::XML::Node]
+      attr_accessor :previous
+
+      # @return [Oga::XML::Node]
+      attr_accessor :next
+
       # @param [Hash] options
       #
       # @option options [Oga::XML::NodeSet] :node_set The node set that this
@@ -27,6 +33,8 @@ module Oga
         @node_set  = set
         @root_node = nil
         @html_p    = nil
+        @previous  = nil
+        @next      = nil
       end
 
       # Returns the child nodes of the current node.
@@ -55,25 +63,6 @@ module Oga
         node_set ? node_set.owner : nil
       end
 
-      # Returns the preceding node, or nil if there is none.
-      #
-      # @return [Oga::XML::Node]
-      def previous
-        index = node_set.index(self) - 1
-
-        index >= 0 ? node_set[index] : nil
-      end
-
-      # Returns the following node, or nil if there is none.
-      #
-      # @return [Oga::XML::Node]
-      def next
-        index  = node_set.index(self) + 1
-        length = node_set.length
-
-        index < length ? node_set[index] : nil
-      end
-
       # Returns the previous element node or nil if there is none.
       #
       # @return [Oga::XML::Element]

lib/oga/xml/node_set.rb

@@ -42,10 +42,10 @@ module Oga
         @owner    = owner
         @existing = {}
 
-        @nodes.each do |node|
+        @nodes.each_with_index do |node, index|
           mark_existing(node)
 
-          take_ownership(node) if @owner
+          take_ownership(node, index) if @owner
         end
       end
 
@@ -98,7 +98,7 @@ module Oga
 
         mark_existing(node)
 
-        take_ownership(node) if @owner
+        take_ownership(node, length - 1) if @owner
       end
 
       alias_method :<<, :push
@@ -113,7 +113,7 @@ module Oga
 
         mark_existing(node)
 
-        take_ownership(node) if @owner
+        take_ownership(node, 0) if @owner
       end
 
       # Shifts a node from the start of the set.
@@ -157,7 +157,7 @@ module Oga
 
         mark_existing(node)
 
-        take_ownership(node) if @owner
+        take_ownership(node, index) if @owner
       end
 
       # Returns the node for the given index.
@@ -224,6 +224,8 @@ module Oga
             sets << node.node_set
 
             node.node_set = nil
+            node.next     = nil
+            node.previous = nil
           end
         end
 
@@ -291,15 +293,34 @@ module Oga
       # set has an owner.
       #
       # @param [Oga::XML::Node] node
-      def take_ownership(node)
+      # @param [Fixnum] index
+      def take_ownership(node, index)
         node.node_set = self
+
+        node.previous = index > 0 ? @nodes[index - 1] : nil
+        node.next = index + 1 < @nodes.length ? @nodes[index + 1] : nil
+
+        node.previous.next = node if node.previous
+        node.next.previous = node if node.next
       end
 
       # Removes ownership of the node if it belongs to the current set.
       #
       # @param [Oga::XML::Node] node
       def remove_ownership(node)
-        node.node_set = nil if node.node_set == self
+        return unless node.node_set == self
+
+        if previous_node = node.previous
+          previous_node.next = node.next
+        end
+
+        if next_node = node.next
+          next_node.previous = node.previous
+        end
+
+        node.node_set = nil
+        node.previous = nil
+        node.next     = nil
       end
 
       # @param [Oga::XML::Node|Oga::XML::Attribute] node

spec/oga/xml/node_set_spec.rb

@@ -25,6 +25,30 @@ describe Oga::XML::NodeSet do
 
       node.node_set.should == set
     end
+
+    it 'sets the previous and next nodes for all nodes owned by the set' do
+      node1 = Oga::XML::Element.new
+      node2 = Oga::XML::Element.new
+      set   = described_class.new([node1, node2], node1)
+
+      node1.next.should     == node2
+      node1.previous.should == nil
+
+      node2.next.should     == nil
+      node2.previous.should == node1
+    end
+
+    it 'does not set the previous and next nodes for nodes that are not owned' do
+      node1 = Oga::XML::Element.new
+      node2 = Oga::XML::Element.new
+      set   = described_class.new([node1, node2])
+
+      node1.previous.should == nil
+      node1.next.should     == nil
+
+      node2.previous.should == nil
+      node2.next.should     == nil
+    end
   end
 
   describe '#each' do
@@ -111,6 +135,22 @@ describe Oga::XML::NodeSet do
 
       child.node_set.should == @set
     end
+
+    it 'updates the previous and next nodes of any owned nodes' do
+      node1 = Oga::XML::Element.new
+      node2 = Oga::XML::Element.new
+
+      @set.owner = node1
+
+      @set.push(node1)
+      @set.push(node2)
+
+      node1.next.should     == node2
+      node1.previous.should == nil
+
+      node2.next.should     == nil
+      node2.previous.should == node1
+    end
   end
 
   describe '#unshift' do
@@ -141,6 +181,24 @@ describe Oga::XML::NodeSet do
 
       child.node_set.should == @set
     end
+
+    it 'updates the next node of the added node' do
+      node = Oga::XML::Element.new
+      @set.owner = node
+
+      @set.unshift(node)
+
+      node.next.should == @n1
+    end
+
+    it 'updates the previous node of the existing node' do
+      node = Oga::XML::Element.new
+      @set.owner = node
+
+      @set.unshift(node)
+
+      @n1.previous.should == node
+    end
   end
 
   describe '#shift' do
@@ -164,6 +222,22 @@ describe Oga::XML::NodeSet do
 
       @n1.node_set.nil?.should == true
     end
+
+    it 'updates the previous and next nodes of the removed node' do
+      @set.shift
+
+      @n1.previous.should == nil
+      @n1.next.should     == nil
+    end
+
+    it 'updates the previous node of the remaining node' do
+      node = Oga::XML::Element.new
+
+      @set.push(node)
+      @set.shift
+
+      node.previous.should == nil
+    end
   end
 
   describe '#pop' do
@@ -187,6 +261,21 @@ describe Oga::XML::NodeSet do
 
       @n1.node_set.nil?.should == true
     end
+
+    it 'updates the previous node of the removed node' do
+      @set.pop
+
+      @n1.previous.should == nil
+    end
+
+    it 'updates the next node of the last remaining node' do
+      node = Oga::XML::Element.new
+
+      @set.push(node)
+      @set.pop
+
+      @n1.next.should == nil
+    end
   end
 
   describe '#insert' do
@@ -230,6 +319,40 @@ describe Oga::XML::NodeSet do
 
       node.node_set.should == @owned_set
     end
+
+    it 'updates the previous and next nodes of the inserted node' do
+      node1 = Oga::XML::Element.new
+      node2 = Oga::XML::Element.new
+      node3 = Oga::XML::Element.new
+
+      @owned_set.push(node1)
+      @owned_set.push(node2)
+
+      @owned_set.insert(1, node3)
+
+      node3.previous.should == node1
+      node3.next.should     == node2
+    end
+
+    it 'updates the next node of the node preceding the inserted node' do
+      node1 = Oga::XML::Element.new
+      node2 = Oga::XML::Element.new
+
+      @owned_set.push(node1)
+      @owned_set.insert(1, node2)
+
+      node1.next.should == node2
+    end
+
+    it 'updates the previous node of the node following the inserted node' do
+      node1 = Oga::XML::Element.new
+      node2 = Oga::XML::Element.new
+
+      @owned_set.push(node1)
+      @owned_set.insert(0, node2)
+
+      node1.previous.should == node2
+    end
   end
 
   describe '#[]' do
@@ -338,6 +461,16 @@ describe Oga::XML::NodeSet do
 
       @doc_set.empty?.should == true
     end
+
+    it 'updates the previous and next nodes for all removed nodes' do
+      @doc_set.remove
+
+      @n1.previous.should == nil
+      @n1.next.should     == nil
+
+      @n2.previous.should == nil
+      @n2.next.should     == nil
+    end
   end
 
   describe '#delete' do
@@ -362,6 +495,16 @@ describe Oga::XML::NodeSet do
 
       @n1.node_set.nil?.should == true
     end
+
+    it 'updates the previous and next nodes of the removed node' do
+      node = Oga::XML::Element.new
+
+      @set.push(node)
+      @set.delete(@n1)
+
+      @n1.previous.should == nil
+      @n1.next.should     == nil
+    end
   end
 
   describe '#attribute' do