From 8f852510fd242ea14ab438352dc889797e9b3714 Mon Sep 17 00:00:00 2001
From: Nolan Waite <nolan@nolanw.ca>
Date: Tue, 2 Feb 2016 20:36:27 -0400
Subject: [PATCH] Implement -addChild: and -removeChild: as conveniences on
 HTMLNode.

Turns out adding an object already in the ordered set calls the KVC method -insertObject:inChildrenAtIndex: with an index one beyond the last index. An NSMutableOrderedSet is documented to do nothing when -addObject: is called with an already-present object. However, by blindly passing along -insertObject:atIndex: to the underlying NSMutableOrderedSet, the object was silently *removed* from the set of children. So now we check for the object's presence and correctly do nothing when it's already present.

Closes #57.
---
 Code/HTMLNode.h       | 10 ++++++++++
 Code/HTMLNode.m       | 24 +++++++++++++++++++++---
 Tests/HTMLNodeTests.m | 28 ++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/Code/HTMLNode.h b/Code/HTMLNode.h
index 90042a3..9d943d8 100644
--- a/Code/HTMLNode.h
+++ b/Code/HTMLNode.h
@@ -39,6 +39,16 @@ NS_ASSUME_NONNULL_BEGIN
 /// Convenience method that returns a mutable proxy for children. The proxy returned by -mutableChildren is much faster than the one obtained by calling -mutableOrderedSetValueForKey: yourself.
 @property (readonly, nonatomic) HTMLMutableOrderedSetOf(HTMLNode *) *mutableChildren;
 
+/**
+    Add a child to the end of the node's set of children, removing it from its current parentNode's set of children. If the child is already in the node's set of children, nothing happens.
+ */
+- (void)addChild:(HTMLNode *)child;
+
+/**
+    Remove a child from the node's set of children. If the child is not in the node's set of children, nothing happens.
+ */
+- (void)removeChild:(HTMLNode *)child;
+
 /**
     The number of nodes that have the node as their parent.
  
diff --git a/Code/HTMLNode.m b/Code/HTMLNode.m
index 8a407aa..f305234 100644
--- a/Code/HTMLNode.m
+++ b/Code/HTMLNode.m
@@ -93,6 +93,20 @@ - (void)removeFromParentNode
     return [[HTMLChildrenRelationshipProxy alloc] initWithNode:self children:_children];
 }
 
+- (void)addChild:(HTMLNode *)child
+{
+    NSParameterAssert(child);
+    
+    [self.mutableChildren addObject:child];
+}
+
+- (void)removeChild:(HTMLNode *)child
+{
+    NSParameterAssert(child);
+    
+    [self.mutableChildren removeObject:child];
+}
+
 - (NSUInteger)numberOfChildren
 {
     return _children.count;
@@ -110,15 +124,19 @@ - (NSUInteger)indexOfChild:(HTMLNode *)child
 
 - (void)insertObject:(HTMLNode *)node inChildrenAtIndex:(NSUInteger)index
 {
+    if ([_children containsObject:node]) {
+        return;
+    }
     [_children insertObject:node atIndex:index];
     [node setParentNode:self updateChildren:NO];
 }
 
 - (void)insertChildren:(NSArray *)array atIndexes:(NSIndexSet *)indexes
 {
-    [_children insertObjects:array atIndexes:indexes];
-    for (HTMLNode *node in array) {
-        [node setParentNode:self updateChildren:NO];
+    NSUInteger nextIndex = indexes.firstIndex;
+    for (HTMLNode *child in array) {
+        [self insertObject:child inChildrenAtIndex:nextIndex];
+        nextIndex = [indexes indexGreaterThanIndex:nextIndex];
     }
 }
 
diff --git a/Tests/HTMLNodeTests.m b/Tests/HTMLNodeTests.m
index 8ce8028..d4baba5 100644
--- a/Tests/HTMLNodeTests.m
+++ b/Tests/HTMLNodeTests.m
@@ -101,6 +101,34 @@ - (void)testNode
     XCTAssertNil(comment.document);
 }
 
+- (void)testAddRemoveChild
+{
+    HTMLComment *comment = [HTMLComment new];
+    
+    XCTAssertNil(comment.document);
+    XCTAssertTrue(_document.children.count == 0);
+    [_document addChild:comment];
+    XCTAssertEqualObjects(comment.document, _document);
+    XCTAssertEqualObjects(_document.children.array, (@[ comment ]));
+    [_document removeChild:comment];
+    XCTAssertNil(comment.document);
+    XCTAssertTrue(_document.children.count == 0);
+    
+    [_document removeChild:comment];
+    XCTAssertTrue(_document.children.count == 0);
+    
+    HTMLElement *element = [HTMLElement new];
+    [_document addChild:comment];
+    [_document addChild:element];
+    XCTAssertEqualObjects(_document.children.array, (@[ comment, element ]));
+    [_document addChild:comment];
+    XCTAssertEqualObjects(_document.children.array, (@[ comment, element ]));
+    
+    HTMLTextNode *text = [HTMLTextNode new];
+    [_document removeChild:text];
+    XCTAssertEqualObjects(_document.children.array, (@[ comment, element ]));
+}
+
 - (void)testRemoveFromParentNode
 {
     HTMLElement *p = [[HTMLElement alloc] initWithTagName:@"p" attributes:nil];