Only store unique nodes in XML::NodeSet.

It's called a "set" after all.

lib/oga/xml/node_set.rb

@@ -1,10 +1,10 @@
 module Oga
   module XML
     ##
-    # The NodeSet class contains a set of {Oga::XML::Node} instances that can
-    # be queried and modified. Optionally NodeSet instances can take ownership
-    # of a node (besides just containing it). This allows the nodes to query
-    # their previous and next elements.
+    # The NodeSet class contains a set of unique {Oga::XML::Node} instances that
+    # can be queried and modified. Optionally NodeSet instances can take
+    # ownership of a node (besides just containing it). This allows the nodes to
+    # query their previous and next elements.
     #
     # There are two types of sets:
     #
@@ -44,7 +44,7 @@ module Oga
       # @param [Oga::XML::NodeSet] owner The owner of the set.
       #
       def initialize(nodes = [], owner = nil)
-        @nodes = nodes
+        @nodes = nodes.uniq
         @owner = owner
 
         @nodes.each { |node| take_ownership(node) }
@@ -105,6 +105,8 @@ module Oga
       # @param [Oga::XML::Node] node
       #
       def push(node)
+        return if @nodes.include?(node)
+
         @nodes << node
 
         take_ownership(node)
@@ -118,6 +120,8 @@ module Oga
       # @param [Oga::XML::Node] node
       #
       def unshift(node)
+        return if @nodes.include?(node)
+
         @nodes.unshift(node)
 
         take_ownership(node)
@@ -176,7 +180,7 @@ module Oga
       # @return [Oga::XML::NodeSet]
       #
       def +(other)
-        return self.class.new(to_a + other.to_a)
+        return self.class.new(to_a | other.to_a)
       end
 
       ##

spec/oga/xml/node_set_spec.rb

@@ -25,6 +25,13 @@ describe Oga::XML::NodeSet do
 
       node.node_set.should == set
     end
+
+    example 'only store unique nodes' do
+      n1  = Oga::XML::Element.new(:name => 'a')
+      set = described_class.new([n1, n1])
+
+      set.length.should == 1
+    end
   end
 
   context '#each' do
@@ -94,6 +101,15 @@ describe Oga::XML::NodeSet do
       @set.length.should == 1
     end
 
+    example 'do not push a node that is already part of the set' do
+      element = Oga::XML::Element.new(:name => 'a')
+
+      @set.push(element)
+      @set.push(element)
+
+      @set.length.should == 1
+    end
+
     example 'take ownership of a node if the set has an owner' do
       child      = Oga::XML::Element.new
       @set.owner = Oga::XML::Element.new
@@ -106,8 +122,8 @@ describe Oga::XML::NodeSet do
 
   context '#unshift' do
     before do
-      n1   = Oga::XML::Element.new(:name => 'a')
-      @set = described_class.new([n1])
+      @n1  = Oga::XML::Element.new(:name => 'a')
+      @set = described_class.new([@n1])
     end
 
     example 'push a node at the beginning of the set' do
@@ -118,6 +134,12 @@ describe Oga::XML::NodeSet do
       @set.first.should == n2
     end
 
+    example 'do not push a node if it is already part of the set' do
+      @set.unshift(@n1)
+
+      @set.length.should == 1
+    end
+
     example 'take ownership of a node if the set has an owner' do
       child      = Oga::XML::Element.new
       @set.owner = Oga::XML::Element.new
@@ -207,6 +229,10 @@ describe Oga::XML::NodeSet do
     example 'merge two sets together' do
       (@set1 + @set2).to_a.should == [@n1, @n2]
     end
+
+    example 'ignore duplicate nodes' do
+      (@set1 + described_class.new([@n1])).length.should == 1
+    end
   end
 
   context '#remove' do