From 5a58b1413767fed4518e8a67c4eb432a31592660 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Sun, 4 Sep 2016 21:07:35 +0200 Subject: [PATCH] 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 | 27 ++----- lib/oga/xml/node_set.rb | 35 +++++++-- spec/oga/xml/node_set_spec.rb | 143 ++++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+), 26 deletions(-) diff --git a/lib/oga/xml/node.rb b/lib/oga/xml/node.rb index 004b236..d64306b 100644 --- a/lib/oga/xml/node.rb +++ b/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] diff --git a/lib/oga/xml/node_set.rb b/lib/oga/xml/node_set.rb index 3535c0f..091c267 100644 --- a/lib/oga/xml/node_set.rb +++ b/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 diff --git a/spec/oga/xml/node_set_spec.rb b/spec/oga/xml/node_set_spec.rb index d82a283..653c710 100644 --- a/spec/oga/xml/node_set_spec.rb +++ b/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