Fixes.

Author: Oren (electricessence)

INode.cs

@@ -2,7 +2,7 @@
 
 namespace Open.Hierarchy
 {
-	public interface INode<TNode> : ICollection<TNode>, IChild<TNode>, IParent<TNode>, IHaveRoot<TNode>
+	public interface INode<TNode> : IList<TNode>, IChild<TNode>, IParent<TNode>, IHaveRoot<TNode>
 		where TNode : INode<TNode>
 	{
 	}

Node.Clone.cs

@@ -31,7 +31,7 @@ public Node<T> Clone(
 			newParentForClone?.Add(clone);
 
 			foreach (var child in _children)
-				clone.Add(child.Clone(clone, onNodeCloned));
+				child.Clone(clone, onNodeCloned);
 
 			clone.Unmapped = Unmapped;
 

Node.Factory.cs

@@ -46,10 +46,11 @@ public Node<T> GetBlankNode()
 			/// Gets a blank node.
 			/// </summary>
 			/// <returns>A blank node.</returns>
-			public Node<T> GetNodeWithValue(T value)
+			public Node<T> GetNodeWithValue(T value, bool asUnmapped = false)
 			{
 				var n = GetBlankNode();
 				n.Value = value;
+				n.Unmapped = asUnmapped;
 				return n;
 			}
 
@@ -95,16 +96,12 @@ static void PrepareForPool(Node<T> n)
 			public Node<T> Map<TRoot>(TRoot root)
 				where TRoot : T
 			{
-				AssertIsAlive();
-
 				var current = GetBlankNode();
 				current.Value = root;
 
 				// Mapping is deferred and occurs on demand.
 				// If values or children change in the node, mapping is disregarded.
 				current.Unmapped = true;
-
-
 				return current;
 			}
 

Node.cs

@@ -64,7 +64,12 @@ IReadOnlyList<Node<T>> EnsureChildrenMapped()
 				if (_value is IParent<T> p)
 				{
 					foreach (var child in p.Children)
-						_children.Add(_factory.Map(child));
+					{
+						var childNode = _factory.Map(child);
+						childNode.Parent = this;
+						_children.Add(childNode);
+					}
+
 				}
 				Unmapped = false;
 			}
@@ -81,50 +86,98 @@ IReadOnlyList<Node<T>> EnsureChildrenMapped()
 
 		// WARNING: Care must be taken not to have duplicate nodes anywhere in the tree but having duplicate values are allowed.
 
+		#region IList<Node<T>> Implementation
+
+		public int IndexOf(Node<T> child)
+		{
+			EnsureChildrenMapped();
+			return _children.IndexOf(child);
+		}
+
+		void AssertOkToJoinFamily(Node<T> child)
+		{
+			if (child.Parent == null) return;
+			if (child.Parent == this)
+				throw new InvalidOperationException("Provided node already belongs to this parent.");
+			throw new InvalidOperationException("Provided node belongs to another parent.");
+		}
+
+		public void Insert(int index, Node<T> child)
+		{
+			if (child == null) throw new ArgumentNullException(nameof(child));
+			Contract.EndContractBlock();
+
+			AssertNotRecycled();
+			AssertOkToJoinFamily(child);
+
+			EnsureChildrenMapped(); // Adding to potentially existing nodes.
+			child.Parent = this;
+			_children.Add(child);
+		}
+
+		// ReSharper disable once UnusedMethodReturnValue.Global
+		public Node<T> RemoveAt(int index)
+		{
+			if (index < 0 && index >= Count) throw new ArgumentOutOfRangeException(nameof(index));
+			Contract.EndContractBlock();
+
+			AssertNotRecycled();
+			var child = this[index];
+			_children.Remove(child);
+
+			Unmapped = false;
+			child.Parent = null; // Need to be very careful about retaining parent references as it may cause a 'leak' per-se.
+			return child;
+		}
+
+		void IList<Node<T>>.RemoveAt(int index)
+			=> RemoveAt(index);
+
+		public Node<T> this[int index]
+		{
+			get => EnsureChildrenMapped()[index];
+			set => Replace(EnsureChildrenMapped()[index], value);
+		}
+
 		#region ICollection<Node<T>> Implementation
 		/// <inheritdoc />
 		public bool IsReadOnly => false;
 
 		/// <inheritdoc />
-		public bool Contains(Node<T> node)
+		public bool Contains(Node<T> child)
 		{
-			if (node == null) throw new ArgumentNullException(nameof(node));
+			if (child == null) throw new ArgumentNullException(nameof(child));
 			Contract.EndContractBlock();
 
-			return _children.Contains(node);
+			return _children.Contains(child);
 		}
 
 		/// <inheritdoc />
-		public bool Remove(Node<T> node)
+		public bool Remove(Node<T> child)
 		{
-			if (node == null) throw new ArgumentNullException(nameof(node));
+			if (child == null) throw new ArgumentNullException(nameof(child));
 			Contract.EndContractBlock();
 
-			if (!_children.Remove(node)) return false;
+			if (!_children.Remove(child)) return false;
 
 			AssertNotRecycled();
 			Unmapped = false;
-			node.Parent = null; // Need to be very careful about retaining parent references as it may cause a 'leak' per-se.
+			child.Parent = null; // Need to be very careful about retaining parent references as it may cause a 'leak' per-se.
 			return true;
 		}
 
 		/// <inheritdoc />
-		public void Add(Node<T> node)
+		public void Add(Node<T> child)
 		{
-			if (node == null) throw new ArgumentNullException(nameof(node));
+			if (child == null) throw new ArgumentNullException(nameof(child));
 			Contract.EndContractBlock();
 
-			if (node.Parent != null)
-			{
-				if (node.Parent == this)
-					throw new InvalidOperationException("Adding a child node more than once.");
-				throw new InvalidOperationException("Adding a node that belongs to another parent.");
-			}
-
 			AssertNotRecycled();
+			AssertOkToJoinFamily(child);
+
 			EnsureChildrenMapped(); // Adding to potentially existing nodes.
-			node.Parent = this;
-			_children.Add(node);
+			child.Parent = this;
+			_children.Add(child);
 		}
 
 		/// <inheritdoc />
@@ -140,7 +193,7 @@ public void Clear()
 		}
 
 		/// <inheritdoc />
-		public int Count => _children.Count;
+		public int Count => EnsureChildrenMapped().Count;
 
 		/// <inheritdoc />
 		public IEnumerator<Node<T>> GetEnumerator()
@@ -157,15 +210,17 @@ public void CopyTo(Node<T>[] array, int arrayIndex)
 			_children.CopyTo(array, arrayIndex);
 		}
 		#endregion
+		#endregion
 
 		/// <summary>
 		/// Gets a new node with the provided value and adds it as a child.
 		/// </summary>
 		/// <param name="value">The value of the new child.</param>
-		public void AddValue(T value)
+		/// <param name="asUnmapped">If true adds the node if it has been prepared for mapping but not yet checked if the value is an IParent.  If value is not an IParent, then this flag does nothing.</param>
+		public void AddValue(T value, bool asUnmapped = false)
 		{
 			AssertNotRecycled();
-			Add(_factory.GetNodeWithValue(value));
+			Add(_factory.GetNodeWithValue(value, asUnmapped));
 		}
 
 		/// <summary>
@@ -278,6 +333,7 @@ public void RecycleChildren()
 			_children.Clear();
 		}
 
+
 	}
 
 }

Open.Hierarchy.csproj

@@ -14,8 +14,9 @@ Part of the "Open" set of libraries.</Description>
     <RepositoryUrl>https://github.com/electricessence/Open.Hierarchy/</RepositoryUrl>
     <RepositoryType>git</RepositoryType>
     <PackageTags>dotnet, dotnetcore, cs, tree, node, hierarchy, parent, child, root</PackageTags>
-    <Version>1.4.4</Version>
-    <PackageReleaseNotes>Added useful node creation methods.</PackageReleaseNotes>
+    <Version>1.5.1</Version>
+    <PackageReleaseNotes>Improved API.  Implemented IList&lt;Node&lt;T&gt;&gt; for all nodes.
+Validated mapping optimization.</PackageReleaseNotes>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|AnyCPU'">