From 08b934a6d6f9d19e7946b7ec8e5645ecaf9168da Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Mon, 22 Aug 2016 08:44:48 +0100
Subject: [PATCH 01/22] pom.xml: upgrade kemitix-parent to 2.0.0

---
 pom.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pom.xml b/pom.xml
index 3071a0d..9c0e8b4 100644
--- a/pom.xml
+++ b/pom.xml
@@ -11,7 +11,7 @@
     <parent>
         <groupId>net.kemitix</groupId>
         <artifactId>kemitix-parent</artifactId>
-        <version>0.6.0</version>
+        <version>2.0.0</version>
     </parent>
 
     <properties>

From acce849819baeb3df94c12e7eb9e09c036f817c4 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Mon, 22 Aug 2016 08:45:00 +0100
Subject: [PATCH 02/22] checkstyle.xml: removed

---
 checkstyle.xml | 192 -------------------------------------------------
 1 file changed, 192 deletions(-)
 delete mode 100644 checkstyle.xml

diff --git a/checkstyle.xml b/checkstyle.xml
deleted file mode 100644
index e54bdb6..0000000
--- a/checkstyle.xml
+++ /dev/null
@@ -1,192 +0,0 @@
-<?xml version="1.0"?>
-<!DOCTYPE module PUBLIC
-          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
-          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
-
-<!--
-
-  Checkstyle configuration that checks the sun coding conventions from:
-
-    - the Java Language Specification at
-      http://java.sun.com/docs/books/jls/second_edition/html/index.html
-
-    - the Sun Code Conventions at http://java.sun.com/docs/codeconv/
-
-    - the Javadoc guidelines at
-      http://java.sun.com/j2se/javadoc/writingdoccomments/index.html
-
-    - the JDK Api documentation http://java.sun.com/j2se/docs/api/index.html
-
-    - some best practices
-
-  Checkstyle is very configurable. Be sure to read the documentation at
-  http://checkstyle.sf.net (or in your downloaded distribution).
-
-  Most Checks are configurable, be sure to consult the documentation.
-
-  To completely disable a check, just comment it out or delete it from the file.
-
-  Finally, it is worth reading the documentation.
-
--->
-
-<module name="Checker">
-    <!--
-        If you set the basedir property below, then all reported file
-        names will be relative to the specified directory. See
-        http://checkstyle.sourceforge.net/5.x/config.html#Checker
-
-        <property name="basedir" value="${basedir}"/>
-    -->
-
-    <!-- Checks that a package-info.java file exists for each package.     -->
-    <!-- See http://checkstyle.sf.net/config_javadoc.html#JavadocPackage -->
-    <module name="JavadocPackage"/>
-
-    <!-- Checks whether files end with a new line.                        -->
-    <!-- See http://checkstyle.sf.net/config_misc.html#NewlineAtEndOfFile -->
-    <module name="NewlineAtEndOfFile"/>
-
-    <!-- Checks that property files contain the same keys.         -->
-    <!-- See http://checkstyle.sf.net/config_misc.html#Translation -->
-    <module name="Translation"/>
-
-    <!-- Checks for Size Violations.                    -->
-    <!-- See http://checkstyle.sf.net/config_sizes.html -->
-    <module name="FileLength"/>
-
-    <!-- Checks for whitespace                               -->
-    <!-- See http://checkstyle.sf.net/config_whitespace.html -->
-    <module name="FileTabCharacter"/>
-
-    <!-- Miscellaneous other checks.                   -->
-    <!-- See http://checkstyle.sf.net/config_misc.html -->
-    <module name="RegexpSingleline">
-        <property name="format" value="\s+$"/>
-        <property name="minimum" value="0"/>
-        <property name="maximum" value="0"/>
-        <property name="message" value="Line has trailing spaces."/>
-    </module>
-
-
-    <!-- Checks for Headers                                -->
-    <!-- See http://checkstyle.sf.net/config_header.html   -->
-    <!-- <module name="Header"> -->
-    <!--   <property name="headerFile" value="${checkstyle.header.file}"/> -->
-    <!--   <property name="fileExtensions" value="java"/> -->
-    <!-- </module> -->
-
-    <module name="TreeWalker">
-
-        <!-- Support @SuppressWarnings annotation -->
-        <!-- See http://checkstyle.sourceforge.net/config.html -->
-        <module name="SuppressWarningsHolder"/>
-
-        <module name="FileContentsHolder"/>
-
-        <!-- Checks for Javadoc comments.                     -->
-        <!-- See http://checkstyle.sf.net/config_javadoc.html -->
-        <module name="JavadocMethod">
-            <property name="scope" value="public"/>
-        </module>
-        <module name="JavadocType"/>
-        <!--<module name="JavadocVariable"/>-->
-        <module name="JavadocStyle"/>
-
-
-        <!-- Checks for Naming Conventions.                  -->
-        <!-- See http://checkstyle.sf.net/config_naming.html -->
-        <module name="ConstantName"/>
-        <module name="LocalFinalVariableName"/>
-        <module name="LocalVariableName"/>
-        <module name="MemberName"/>
-        <module name="MethodName"/>
-        <module name="PackageName"/>
-        <module name="ParameterName"/>
-        <module name="StaticVariableName"/>
-        <module name="TypeName"/>
-
-
-        <!-- Checks for imports                              -->
-        <!-- See http://checkstyle.sf.net/config_import.html -->
-        <module name="AvoidStarImport"/>
-        <module name="IllegalImport"/> <!-- defaults to sun.* packages -->
-        <module name="RedundantImport"/>
-        <module name="UnusedImports"/>
-
-
-        <!-- Checks for Size Violations.                    -->
-        <!-- See http://checkstyle.sf.net/config_sizes.html -->
-        <module name="LineLength"/>
-        <module name="MethodLength"/>
-        <module name="ParameterNumber"/>
-
-
-        <!-- Checks for whitespace                               -->
-        <!-- See http://checkstyle.sf.net/config_whitespace.html -->
-        <module name="EmptyForIteratorPad"/>
-        <module name="GenericWhitespace"/>
-        <module name="MethodParamPad"/>
-        <module name="NoWhitespaceAfter"/>
-        <module name="NoWhitespaceBefore"/>
-        <module name="OperatorWrap"/>
-        <module name="ParenPad"/>
-        <module name="TypecastParenPad"/>
-        <module name="WhitespaceAfter"/>
-        <module name="WhitespaceAround"/>
-
-
-        <!-- Modifier Checks                                    -->
-        <!-- See http://checkstyle.sf.net/config_modifiers.html -->
-        <module name="ModifierOrder"/>
-        <module name="RedundantModifier"/>
-
-
-        <!-- Checks for blocks. You know, those {}'s         -->
-        <!-- See http://checkstyle.sf.net/config_blocks.html -->
-        <module name="AvoidNestedBlocks"/>
-        <module name="EmptyBlock"/>
-        <module name="LeftCurly"/>
-        <module name="NeedBraces"/>
-        <module name="RightCurly"/>
-
-
-        <!-- Checks for common coding problems               -->
-        <!-- See http://checkstyle.sf.net/config_coding.html -->
-        <module name="AvoidInlineConditionals"/>
-        <module name="EmptyStatement"/>
-        <module name="EqualsHashCode"/>
-        <module name="HiddenField">
-            <property name="ignoreConstructorParameter" value="true"/>
-            <property name="ignoreSetter" value="true"/>
-            <property name="setterCanReturnItsClass" value="true"/>
-        </module>
-        <module name="IllegalInstantiation"/>
-        <module name="InnerAssignment"/>
-        <module name="MagicNumber"/>
-        <module name="MissingSwitchDefault"/>
-        <module name="SimplifyBooleanExpression"/>
-        <module name="SimplifyBooleanReturn"/>
-
-        <!-- Checks for class design                         -->
-        <!-- See http://checkstyle.sf.net/config_design.html -->
-        <!--<module name="DesignForExtension"/>-->
-        <module name="FinalClass"/>
-        <module name="HideUtilityClassConstructor"/>
-        <module name="InterfaceIsType"/>
-        <module name="VisibilityModifier"/>
-
-
-        <!-- Miscellaneous other checks.                   -->
-        <!-- See http://checkstyle.sf.net/config_misc.html -->
-        <module name="ArrayTypeStyle"/>
-        <module name="FinalParameters"/>
-        <module name="TodoComment"/>
-        <module name="UpperEll"/>
-
-    </module>
-
-    <module name="SuppressWarningsFilter"/>
-    <module name="SuppressionCommentFilter"/>
-
-</module>
\ No newline at end of file

From e108d61ac51c0c66eb62a575fe0acafa017b793c Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Mon, 22 Aug 2016 08:48:31 +0100
Subject: [PATCH 03/22] pom.xml: move lombok to compile scope

---
 pom.xml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pom.xml b/pom.xml
index 9c0e8b4..a502cc3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -50,7 +50,6 @@
             <groupId>org.projectlombok</groupId>
             <artifactId>lombok</artifactId>
             <version>1.16.8</version>
-            <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>junit</groupId>

From d99fcdcebeb8e8661af754923d5100591ca45185 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Mon, 22 Aug 2016 08:51:18 +0100
Subject: [PATCH 04/22] NodeItem: replace manual null checks with @NonNull

---
 src/main/java/net/kemitix/node/NodeItem.java | 42 +++++---------------
 1 file changed, 10 insertions(+), 32 deletions(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index e688355..3f6f6bc 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -1,5 +1,7 @@
 package net.kemitix.node;
 
+import lombok.NonNull;
+
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
@@ -128,10 +130,7 @@ class NodeItem<T> implements Node<T> {
      * @param child the node to add
      */
     @Override
-    public void addChild(final Node<T> child) {
-        if (child == null) {
-            throw new NullPointerException("child");
-        }
+    public void addChild(@NonNull final Node<T> child) {
         if (this.equals(child) || isDescendantOf(child)) {
             throw new NodeException("Child is an ancestor");
         }
@@ -162,10 +161,7 @@ class NodeItem<T> implements Node<T> {
      * @return the new child node
      */
     @Override
-    public Node<T> createChild(final T child) {
-        if (child == null) {
-            throw new NullPointerException("child");
-        }
+    public Node<T> createChild(@NonNull final T child) {
         return new NodeItem<>(child, this);
     }
 
@@ -184,10 +180,7 @@ class NodeItem<T> implements Node<T> {
      * @param descendants the line of descendants from the current node
      */
     @Override
-    public void createDescendantLine(final List<T> descendants) {
-        if (descendants == null) {
-            throw new NullPointerException("descendants");
-        }
+    public void createDescendantLine(@NonNull final List<T> descendants) {
         if (!descendants.isEmpty()) {
             findOrCreateChild(descendants.get(0)).createDescendantLine(
                     descendants.subList(1, descendants.size()));
@@ -204,10 +197,7 @@ class NodeItem<T> implements Node<T> {
      */
     @Override
     @Deprecated
-    public Node<T> findOrCreateChild(final T child) {
-        if (child == null) {
-            throw new NullPointerException("child");
-        }
+    public Node<T> findOrCreateChild(@NonNull final T child) {
         return findChild(child).orElseGet(() -> createChild(child));
     }
 
@@ -219,10 +209,7 @@ class NodeItem<T> implements Node<T> {
      * @return an {@link Optional} containing the child node if found
      */
     @Override
-    public Optional<Node<T>> findChild(final T child) {
-        if (child == null) {
-            throw new NullPointerException("child");
-        }
+    public Optional<Node<T>> findChild(@NonNull final T child) {
         return children.stream().filter(node -> {
             final Optional<T> d = node.getData();
             return d.isPresent() && d.get().equals(child);
@@ -257,10 +244,7 @@ class NodeItem<T> implements Node<T> {
      * @param parent the new parent node
      */
     @Override
-    public final void setParent(final Node<T> parent) {
-        if (parent == null) {
-            throw new NullPointerException("parent");
-        }
+    public final void setParent(@NonNull final Node<T> parent) {
         if (this.equals(parent) || parent.isDescendantOf(this)) {
             throw new NodeException("Parent is a descendant");
         }
@@ -279,10 +263,7 @@ class NodeItem<T> implements Node<T> {
      * @return the child or null
      */
     @Override
-    public Optional<Node<T>> findInPath(final List<T> path) {
-        if (path == null) {
-            throw new NullPointerException("path");
-        }
+    public Optional<Node<T>> findInPath(@NonNull final List<T> path) {
         if (path.size() > 0) {
             Optional<Node<T>> found = findChild(path.get(0));
             if (found.isPresent()) {
@@ -330,10 +311,7 @@ class NodeItem<T> implements Node<T> {
     }
 
     @Override
-    public Optional<Node<T>> findChildByName(final String named) {
-        if (named == null) {
-            throw new NullPointerException("name");
-        }
+    public Optional<Node<T>> findChildByName(@NonNull final String named) {
         return children.stream()
                        .filter((Node<T> t) -> t.getName().equals(named))
                        .findAny();

From b2130442e50a67f7bd58e8e7b8c702ad58acb713 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Mon, 22 Aug 2016 08:52:42 +0100
Subject: [PATCH 05/22] AbstractNodeItem: replace manual null checks with
 @NonNull

---
 .../java/net/kemitix/node/AbstractNodeItem.java | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/src/main/java/net/kemitix/node/AbstractNodeItem.java b/src/main/java/net/kemitix/node/AbstractNodeItem.java
index 6aa90e7..4815d93 100644
--- a/src/main/java/net/kemitix/node/AbstractNodeItem.java
+++ b/src/main/java/net/kemitix/node/AbstractNodeItem.java
@@ -1,5 +1,7 @@
 package net.kemitix.node;
 
+import lombok.NonNull;
+
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
@@ -65,10 +67,7 @@ abstract class AbstractNodeItem<T> implements Node<T> {
      * @return an {@link Optional} containing the child node if found
      */
     @Override
-    public Optional<Node<T>> findChild(final T child) {
-        if (child == null) {
-            throw new NullPointerException("child");
-        }
+    public Optional<Node<T>> findChild(@NonNull final T child) {
         return children.stream().filter(node -> {
             final Optional<T> d = node.getData();
             return d.isPresent() && d.get().equals(child);
@@ -102,10 +101,7 @@ abstract class AbstractNodeItem<T> implements Node<T> {
      * @return the child or null
      */
     @Override
-    public Optional<Node<T>> findInPath(final List<T> path) {
-        if (path == null) {
-            throw new NullPointerException("path");
-        }
+    public Optional<Node<T>> findInPath(@NonNull final List<T> path) {
         if (path.size() > 0) {
             Optional<Node<T>> found = findChild(path.get(0));
             if (found.isPresent()) {
@@ -119,10 +115,7 @@ abstract class AbstractNodeItem<T> implements Node<T> {
     }
 
     @Override
-    public Optional<Node<T>> findChildByName(final String named) {
-        if (named == null) {
-            throw new NullPointerException("name");
-        }
+    public Optional<Node<T>> findChildByName(@NonNull final String named) {
         return children.stream()
                        .filter(n -> n.getName().equals(named))
                        .findAny();

From 2a435a849e74add511897a2545ccf37433ab368d Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Mon, 22 Aug 2016 08:58:58 +0100
Subject: [PATCH 06/22] NodeItem: refactor formatting name by depth

Remove duplicate string literals.
---
 src/main/java/net/kemitix/node/NodeItem.java | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index 3f6f6bc..3afd79a 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -331,17 +331,18 @@ class NodeItem<T> implements Node<T> {
         final StringBuilder sb = new StringBuilder();
         final String unnamed = "(unnamed)";
         if (isNamed()) {
-            sb.append(String.format("[%1$" + (depth + name.length()) + "s]\n",
-                    name));
+            sb.append(formatByDepth(name, depth));
         } else if (!children.isEmpty()) {
-            sb.append(
-                    String.format("[%1$" + (depth + unnamed.length()) + "s]\n",
-                            unnamed));
+            sb.append(formatByDepth(unnamed, depth));
         }
         getChildren().stream().forEach(c -> sb.append(c.drawTree(depth + 1)));
         return sb.toString();
     }
 
+    private String formatByDepth(final String value, final int depth) {
+        return String.format("[%1$" + (depth + value.length()) + "s]\n", value);
+    }
+
     @Override
     public boolean isNamed() {
         String currentName = getName();

From 6253a3226f6d7bd1a6f9559a215a762a5bc741b5 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Mon, 22 Aug 2016 08:59:52 +0100
Subject: [PATCH 07/22] NodeItem: simplify stream().forEach()

---
 src/main/java/net/kemitix/node/NodeItem.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index 3afd79a..caa23e0 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -335,7 +335,7 @@ class NodeItem<T> implements Node<T> {
         } else if (!children.isEmpty()) {
             sb.append(formatByDepth(unnamed, depth));
         }
-        getChildren().stream().forEach(c -> sb.append(c.drawTree(depth + 1)));
+        getChildren().forEach(c -> sb.append(c.drawTree(depth + 1)));
         return sb.toString();
     }
 

From 3d94eaeb32647c8360d4185809a15b6384220ed2 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Mon, 22 Aug 2016 09:01:34 +0100
Subject: [PATCH 08/22] AbstractNodeItem: refactor formatting name by depth

Remove duplicate string literals.
---
 src/main/java/net/kemitix/node/AbstractNodeItem.java | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/main/java/net/kemitix/node/AbstractNodeItem.java b/src/main/java/net/kemitix/node/AbstractNodeItem.java
index 4815d93..f177c73 100644
--- a/src/main/java/net/kemitix/node/AbstractNodeItem.java
+++ b/src/main/java/net/kemitix/node/AbstractNodeItem.java
@@ -132,17 +132,18 @@ abstract class AbstractNodeItem<T> implements Node<T> {
         final StringBuilder sb = new StringBuilder();
         final String unnamed = "(unnamed)";
         if (isNamed()) {
-            sb.append(String.format("[%1$" + (depth + name.length()) + "s]\n",
-                    name));
+            sb.append(formatByDepth(name, depth));
         } else if (!children.isEmpty()) {
-            sb.append(
-                    String.format("[%1$" + (depth + unnamed.length()) + "s]\n",
-                            unnamed));
+            sb.append(formatByDepth(unnamed, depth));
         }
         getChildren().forEach(c -> sb.append(c.drawTree(depth + 1)));
         return sb.toString();
     }
 
+    private String formatByDepth(final String value, final int depth) {
+        return String.format("[%1$" + (depth + value.length()) + "s]\n", value);
+    }
+
     @Override
     public boolean isNamed() {
         return name != null && name.length() > 0;

From 5c129b54b8af4a127ef3eb30b7b209f97990ada2 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Mon, 22 Aug 2016 09:04:16 +0100
Subject: [PATCH 09/22] NodeItem: put trailing comments on their own line

---
 src/main/java/net/kemitix/node/NodeItem.java | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index caa23e0..e5db2fd 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -279,13 +279,15 @@ class NodeItem<T> implements Node<T> {
     @Override
     public void insertInPath(final Node<T> nodeItem, final String... path) {
         if (path.length == 0) {
-            if (!nodeItem.isNamed()) { // nothing to conflict with
+            if (!nodeItem.isNamed()) {
+                // nothing to conflict with
                 addChild(nodeItem);
                 return;
             }
             String nodeName = nodeItem.getName();
             final Optional<Node<T>> childNamed = findChildByName(nodeName);
-            if (!childNamed.isPresent()) { // nothing with the same name exists
+            if (!childNamed.isPresent()) {
+                // nothing with the same name exists
                 addChild(nodeItem);
                 return;
             }

From 37247e93bc28ae8b0a36df8a7c1797c548192121 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 18:44:29 +0100
Subject: [PATCH 10/22] node: change javadoc element order to Atclause order

The default configuration of the AtclauseOrder is a bit strange.
---
 src/main/java/net/kemitix/node/AbstractNodeItem.java  | 4 ++--
 src/main/java/net/kemitix/node/ImmutableNodeItem.java | 4 ++--
 src/main/java/net/kemitix/node/Node.java              | 4 ++--
 src/main/java/net/kemitix/node/NodeItem.java          | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/main/java/net/kemitix/node/AbstractNodeItem.java b/src/main/java/net/kemitix/node/AbstractNodeItem.java
index f177c73..f7efaf9 100644
--- a/src/main/java/net/kemitix/node/AbstractNodeItem.java
+++ b/src/main/java/net/kemitix/node/AbstractNodeItem.java
@@ -11,9 +11,9 @@ import java.util.Set;
  * An abstract node item, providing default implementations for most read-only
  * operations.
  *
- * @param <T> the type of data stored in each node
+ * @author Paul Campbell
  *
- * @author pcampbell
+ * @param <T> the type of data stored in each node
  */
 abstract class AbstractNodeItem<T> implements Node<T> {
 
diff --git a/src/main/java/net/kemitix/node/ImmutableNodeItem.java b/src/main/java/net/kemitix/node/ImmutableNodeItem.java
index 7b80a59..dbc85ec 100644
--- a/src/main/java/net/kemitix/node/ImmutableNodeItem.java
+++ b/src/main/java/net/kemitix/node/ImmutableNodeItem.java
@@ -11,9 +11,9 @@ import java.util.Set;
  * getData()} they could then modify the original data within the node. This
  * wouldn't affect the integrity of the node tree structure, however.</p>
  *
- * @param <T> the type of data stored in each node
+ * @author Paul Campbell
  *
- * @author pcampbell
+ * @param <T> the type of data stored in each node
  */
 final class ImmutableNodeItem<T> extends AbstractNodeItem<T> {
 
diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java
index 7cda730..f1fa7ba 100644
--- a/src/main/java/net/kemitix/node/Node.java
+++ b/src/main/java/net/kemitix/node/Node.java
@@ -7,9 +7,9 @@ import java.util.Set;
 /**
  * An interface for tree node items.
  *
- * @param <T> the type of data held in each node
+ * @author Paul Campbell
  *
- * @author pcampbell
+ * @param <T> the type of data held in each node
  */
 public interface Node<T> {
 
diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index e5db2fd..812adf4 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -12,9 +12,9 @@ import java.util.function.Function;
 /**
  * Represents a tree of nodes.
  *
- * @param <T> the type of data stored in each node
+ * @author Paul Campbell
  *
- * @author pcampbell
+ * @param <T> the type of data stored in each node
  */
 class NodeItem<T> implements Node<T> {
 

From 9f3aec202af31a3ef83f59da503e3ac0de099c40 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 18:59:22 +0100
Subject: [PATCH 11/22] {Abstract}NodeItem: reduce nested if statements

---
 .../net/kemitix/node/AbstractNodeItem.java    | 15 ++++-----
 src/main/java/net/kemitix/node/NodeItem.java  | 31 +++++++++----------
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/src/main/java/net/kemitix/node/AbstractNodeItem.java b/src/main/java/net/kemitix/node/AbstractNodeItem.java
index f7efaf9..3097d4a 100644
--- a/src/main/java/net/kemitix/node/AbstractNodeItem.java
+++ b/src/main/java/net/kemitix/node/AbstractNodeItem.java
@@ -102,14 +102,15 @@ abstract class AbstractNodeItem<T> implements Node<T> {
      */
     @Override
     public Optional<Node<T>> findInPath(@NonNull final List<T> path) {
-        if (path.size() > 0) {
-            Optional<Node<T>> found = findChild(path.get(0));
-            if (found.isPresent()) {
-                if (path.size() > 1) {
-                    return found.get().findInPath(path.subList(1, path.size()));
-                }
-                return found;
+        if (path.isEmpty()) {
+            return Optional.empty();
+        }
+        Optional<Node<T>> found = findChild(path.get(0));
+        if (found.isPresent()) {
+            if (path.size() > 1) {
+                return found.get().findInPath(path.subList(1, path.size()));
             }
+            return found;
         }
         return Optional.empty();
     }
diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index 812adf4..2ed9b02 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -2,19 +2,15 @@ package net.kemitix.node;
 
 import lombok.NonNull;
 
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Optional;
-import java.util.Set;
+import java.util.*;
 import java.util.function.Function;
 
 /**
  * Represents a tree of nodes.
  *
- * @author Paul Campbell
- *
  * @param <T> the type of data stored in each node
+ *
+ * @author Paul Campbell
  */
 class NodeItem<T> implements Node<T> {
 
@@ -147,7 +143,7 @@ class NodeItem<T> implements Node<T> {
         Optional<Node<T>> childParent = child.getParent();
         boolean isOrphan = !childParent.isPresent();
         boolean hasDifferentParent = !isOrphan && !childParent.get()
-                                                              .equals(this);
+                .equals(this);
         if (isOrphan || hasDifferentParent) {
             child.setParent(this);
         }
@@ -264,14 +260,15 @@ class NodeItem<T> implements Node<T> {
      */
     @Override
     public Optional<Node<T>> findInPath(@NonNull final List<T> path) {
-        if (path.size() > 0) {
-            Optional<Node<T>> found = findChild(path.get(0));
-            if (found.isPresent()) {
-                if (path.size() > 1) {
-                    return found.get().findInPath(path.subList(1, path.size()));
-                }
-                return found;
+        if (path.isEmpty()) {
+            return Optional.empty();
+        }
+        Optional<Node<T>> found = findChild(path.get(0));
+        if (found.isPresent()) {
+            if (path.size() > 1) {
+                return found.get().findInPath(path.subList(1, path.size()));
             }
+            return found;
         }
         return Optional.empty();
     }
@@ -315,8 +312,8 @@ class NodeItem<T> implements Node<T> {
     @Override
     public Optional<Node<T>> findChildByName(@NonNull final String named) {
         return children.stream()
-                       .filter((Node<T> t) -> t.getName().equals(named))
-                       .findAny();
+                .filter((Node<T> t) -> t.getName().equals(named))
+                .findAny();
     }
 
     @Override

From 02d07605c0f6e818298869b4c55d37d3d32bd98d Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 19:20:16 +0100
Subject: [PATCH 12/22] NodeFindInPathTestsCategory: added

---
 .../net/kemitix/node/NodeFindInPathTestsCategory.java    | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 src/test/java/net/kemitix/node/NodeFindInPathTestsCategory.java

diff --git a/src/test/java/net/kemitix/node/NodeFindInPathTestsCategory.java b/src/test/java/net/kemitix/node/NodeFindInPathTestsCategory.java
new file mode 100644
index 0000000..979190f
--- /dev/null
+++ b/src/test/java/net/kemitix/node/NodeFindInPathTestsCategory.java
@@ -0,0 +1,9 @@
+package net.kemitix.node;
+
+/**
+ * Category marker for tests relating to implementations of Node.findInPath(...).
+ *
+ * @author Paul Campbell
+ */
+public interface NodeFindInPathTestsCategory {
+}

From 6987f927fe4e29a0c8f1594e0a0558554f8b620f Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 19:21:18 +0100
Subject: [PATCH 13/22] {Immutable}NodeItemTest: categorise findInPath tests

---
 .../java/net/kemitix/node/ImmutableNodeItemTest.java     | 9 +++++++--
 src/test/java/net/kemitix/node/NodeItemTest.java         | 9 +++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/test/java/net/kemitix/node/ImmutableNodeItemTest.java b/src/test/java/net/kemitix/node/ImmutableNodeItemTest.java
index 9475ff5..91eee8a 100644
--- a/src/test/java/net/kemitix/node/ImmutableNodeItemTest.java
+++ b/src/test/java/net/kemitix/node/ImmutableNodeItemTest.java
@@ -4,14 +4,15 @@ import lombok.val;
 import org.assertj.core.api.SoftAssertions;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.experimental.categories.Category;
 import org.junit.rules.ExpectedException;
 
-import static org.assertj.core.api.Assertions.assertThat;
-
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Optional;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 /**
  * Test for {@link ImmutableNodeItem}.
  *
@@ -141,6 +142,7 @@ public class ImmutableNodeItemTest {
      * Test that we can walk a tree to the target node.
      */
     @Test
+    @Category(NodeFindInPathTestsCategory.class)
     public void shouldWalkTreeToNode() {
         //given
         val root = Nodes.unnamedRoot("root");
@@ -160,6 +162,7 @@ public class ImmutableNodeItemTest {
      * doesn't exist.
      */
     @Test
+    @Category(NodeFindInPathTestsCategory.class)
     public void shouldNotFindNonExistentChildNode() {
         //given
         val root = Nodes.unnamedRoot("root");
@@ -176,6 +179,7 @@ public class ImmutableNodeItemTest {
      * Test that when we pass null we get an exception.
      */
     @Test
+    @Category(NodeFindInPathTestsCategory.class)
     public void shouldThrowNEWhenWalkTreeNull() {
         //given
         immutableNode = Nodes.asImmutable(Nodes.unnamedRoot("subject"));
@@ -190,6 +194,7 @@ public class ImmutableNodeItemTest {
      * a result.
      */
     @Test
+    @Category(NodeFindInPathTestsCategory.class)
     public void shouldReturnEmptyForEmptyWalkTreePath() {
         //given
         immutableNode = Nodes.asImmutable(Nodes.unnamedRoot("subject"));
diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java
index 247e7fb..2702ae0 100644
--- a/src/test/java/net/kemitix/node/NodeItemTest.java
+++ b/src/test/java/net/kemitix/node/NodeItemTest.java
@@ -4,14 +4,15 @@ import lombok.val;
 import org.assertj.core.api.SoftAssertions;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.experimental.categories.Category;
 import org.junit.rules.ExpectedException;
 
-import static org.assertj.core.api.Assertions.assertThat;
-
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Optional;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 /**
  * Test for {@link NodeItem}.
  *
@@ -309,6 +310,7 @@ public class NodeItemTest {
      * Test that we can walk a tree to the target node.
      */
     @Test
+    @Category(NodeFindInPathTestsCategory.class)
     public void shouldWalkTreeToNode() {
         //given
         val grandparent = "grandparent";
@@ -334,6 +336,7 @@ public class NodeItemTest {
      * doesn't exist.
      */
     @Test
+    @Category(NodeFindInPathTestsCategory.class)
     public void shouldNotFindNonExistentChildNode() {
         //given
         val parent = "parent";
@@ -352,6 +355,7 @@ public class NodeItemTest {
      * Test that when we pass null we get an exception.
      */
     @Test
+    @Category(NodeFindInPathTestsCategory.class)
     public void shouldThrowNEWhenWalkTreeNull() {
         //given
         node = Nodes.unnamedRoot("subject");
@@ -366,6 +370,7 @@ public class NodeItemTest {
      * a result.
      */
     @Test
+    @Category(NodeFindInPathTestsCategory.class)
     public void shouldReturnEmptyForEmptyWalkTreePath() {
         //given
         node = Nodes.unnamedRoot("subject");

From 91c57f098e3feca06c2ca91d4af6c35db5838166 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 19:21:41 +0100
Subject: [PATCH 14/22] {Abstract}NodeItem: rewrite findInPath to avoid
 recursion

---
 .../java/net/kemitix/node/AbstractNodeItem.java   | 15 +++++++++------
 src/main/java/net/kemitix/node/NodeItem.java      | 15 +++++++++------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/main/java/net/kemitix/node/AbstractNodeItem.java b/src/main/java/net/kemitix/node/AbstractNodeItem.java
index 3097d4a..1fe6eab 100644
--- a/src/main/java/net/kemitix/node/AbstractNodeItem.java
+++ b/src/main/java/net/kemitix/node/AbstractNodeItem.java
@@ -105,14 +105,17 @@ abstract class AbstractNodeItem<T> implements Node<T> {
         if (path.isEmpty()) {
             return Optional.empty();
         }
-        Optional<Node<T>> found = findChild(path.get(0));
-        if (found.isPresent()) {
-            if (path.size() > 1) {
-                return found.get().findInPath(path.subList(1, path.size()));
+        Node<T> current = this;
+        for (T item : path) {
+            final Optional<Node<T>> child = current.findChild(item);
+            if (child.isPresent()) {
+                current = child.get();
+            } else {
+                current = null;
+                break;
             }
-            return found;
         }
-        return Optional.empty();
+        return Optional.ofNullable(current);
     }
 
     @Override
diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index 2ed9b02..519867f 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -263,14 +263,17 @@ class NodeItem<T> implements Node<T> {
         if (path.isEmpty()) {
             return Optional.empty();
         }
-        Optional<Node<T>> found = findChild(path.get(0));
-        if (found.isPresent()) {
-            if (path.size() > 1) {
-                return found.get().findInPath(path.subList(1, path.size()));
+        Node<T> current = this;
+        for (T item : path) {
+            final Optional<Node<T>> child = current.findChild(item);
+            if (child.isPresent()) {
+                current = child.get();
+            } else {
+                current = null;
+                break;
             }
-            return found;
         }
-        return Optional.empty();
+        return Optional.ofNullable(current);
     }
 
     @Override

From e28b140db82465965ae188ed3c5274a97847ee04 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 19:28:20 +0100
Subject: [PATCH 15/22] NodeItem: simplify inserting child into path

---
 src/main/java/net/kemitix/node/NodeItem.java | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index 519867f..067ef5e 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -301,15 +301,10 @@ class NodeItem<T> implements Node<T> {
             }
             return;
         }
-        String item = path[0];
-        final Optional<Node<T>> childNamed = findChildByName(item);
-        Node<T> child;
-        if (!childNamed.isPresent()) {
-            child = new NodeItem<>(null, item, this);
-        } else {
-            child = childNamed.get();
-        }
-        child.insertInPath(nodeItem, Arrays.copyOfRange(path, 1, path.length));
+        val item = path[0];
+        findChildByName(item)
+                .orElseGet(() -> new NodeItem<>(null, item, this))
+                .insertInPath(nodeItem, Arrays.copyOfRange(path, 1, path.length));
     }
 
     @Override

From 40f49fd832db73cf8fd04260e4b4d249b8334dd6 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 19:40:08 +0100
Subject: [PATCH 16/22] NodeItem: refactored insertInPath to be easier to
 understand

---
 src/main/java/net/kemitix/node/NodeItem.java | 59 +++++++++++---------
 1 file changed, 34 insertions(+), 25 deletions(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index 067ef5e..6c5f241 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -1,6 +1,7 @@
 package net.kemitix.node;
 
 import lombok.NonNull;
+import lombok.val;
 
 import java.util.*;
 import java.util.function.Function;
@@ -279,32 +280,40 @@ class NodeItem<T> implements Node<T> {
     @Override
     public void insertInPath(final Node<T> nodeItem, final String... path) {
         if (path.length == 0) {
-            if (!nodeItem.isNamed()) {
-                // nothing to conflict with
-                addChild(nodeItem);
-                return;
-            }
-            String nodeName = nodeItem.getName();
-            final Optional<Node<T>> childNamed = findChildByName(nodeName);
-            if (!childNamed.isPresent()) {
-                // nothing with the same name exists
-                addChild(nodeItem);
-                return;
-            }
-            // we have an existing node with the same name
-            final Node<T> existing = childNamed.get();
-            if (!existing.isEmpty()) {
-                throw new NodeException("A non-empty node named '" + nodeName
-                        + "' already exists here");
-            } else {
-                nodeItem.getData().ifPresent(existing::setData);
-            }
-            return;
+            insertChild(nodeItem);
+        } else {
+            val item = path[0];
+            findChildByName(item)
+                    .orElseGet(() -> new NodeItem<>(null, item, this))
+                    .insertInPath(nodeItem, Arrays.copyOfRange(path, 1, path.length));
+        }
+    }
+
+    private void insertChild(final Node<T> nodeItem) {
+        if (nodeItem.isNamed()) {
+            insertNamedChild(nodeItem);
+        } else {
+            // nothing to conflict with
+            addChild(nodeItem);
+        }
+    }
+
+    private void insertNamedChild(final Node<T> nodeItem) {
+        val childByName = findChildByName(nodeItem.getName());
+        if (childByName.isPresent()) {
+            // we have an existing node with the same name
+            val existing = childByName.get();
+            if (existing.isEmpty()) {
+                // place any data in the new node into the existing empty node
+                nodeItem.getData().ifPresent(existing::setData);
+            } else {
+                throw new NodeException("A non-empty node named '" + nodeItem.getName()
+                        + "' already exists here");
+            }
+        } else {
+            // nothing with the same name exists
+            addChild(nodeItem);
         }
-        val item = path[0];
-        findChildByName(item)
-                .orElseGet(() -> new NodeItem<>(null, item, this))
-                .insertInPath(nodeItem, Arrays.copyOfRange(path, 1, path.length));
     }
 
     @Override

From 1da9d44a8b70f879bb17b35c242b2d7d645c066d Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 19:45:16 +0100
Subject: [PATCH 17/22] Node: add javadoc @deprecated to findOrCreateChild

---
 src/main/java/net/kemitix/node/Node.java     | 3 +++
 src/main/java/net/kemitix/node/NodeItem.java | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java
index f1fa7ba..395ebb7 100644
--- a/src/main/java/net/kemitix/node/Node.java
+++ b/src/main/java/net/kemitix/node/Node.java
@@ -113,6 +113,9 @@ public interface Node<T> {
      * @param child the child's data to search or create with
      *
      * @return the found or created child node
+     *
+     * @deprecated use node.findChild(child).orElseGet(() ->
+     * node.createChild(child));
      */
     @Deprecated
     Node<T> findOrCreateChild(T child);
diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index 6c5f241..c41a3d7 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -191,6 +191,9 @@ class NodeItem<T> implements Node<T> {
      * @param child the child's data to search or create with
      *
      * @return the found or created child node
+     *
+     * @deprecated use node.findChild(child).orElseGet(() -> node.createChild
+     * (child));
      */
     @Override
     @Deprecated

From cd1afc6778b071fb3faba39d761f518d2ff66c20 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 19:46:30 +0100
Subject: [PATCH 18/22] NodeItem: avoid import .*

---
 src/main/java/net/kemitix/node/NodeItem.java | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index c41a3d7..7f0cd75 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -3,7 +3,11 @@ package net.kemitix.node;
 import lombok.NonNull;
 import lombok.val;
 
-import java.util.*;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
 import java.util.function.Function;
 
 /**

From e4c4fdf4bc2bb12cfeeddc6002a1b992855ef5ab Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 19:47:19 +0100
Subject: [PATCH 19/22] NodeItem,: use strange javadoc element ordering

---
 src/main/java/net/kemitix/node/NodeItem.java | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index 7f0cd75..2ee8b97 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -13,9 +13,9 @@ import java.util.function.Function;
 /**
  * Represents a tree of nodes.
  *
- * @param <T> the type of data stored in each node
- *
  * @author Paul Campbell
+ *
+ * @param <T> the type of data stored in each node
  */
 class NodeItem<T> implements Node<T> {
 

From 9ec2668802d34aed90d84384e31a3256b57e5839 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 19:49:13 +0100
Subject: [PATCH 20/22] NodeItem: wrap lines at 80 columns

---
 src/main/java/net/kemitix/node/NodeItem.java | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index 2ee8b97..979fddd 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -292,7 +292,8 @@ class NodeItem<T> implements Node<T> {
             val item = path[0];
             findChildByName(item)
                     .orElseGet(() -> new NodeItem<>(null, item, this))
-                    .insertInPath(nodeItem, Arrays.copyOfRange(path, 1, path.length));
+                    .insertInPath(nodeItem,
+                            Arrays.copyOfRange(path, 1, path.length));
         }
     }
 
@@ -314,8 +315,8 @@ class NodeItem<T> implements Node<T> {
                 // place any data in the new node into the existing empty node
                 nodeItem.getData().ifPresent(existing::setData);
             } else {
-                throw new NodeException("A non-empty node named '" + nodeItem.getName()
-                        + "' already exists here");
+                throw new NodeException("A non-empty node named '"
+                        + nodeItem.getName() + "' already exists here");
             }
         } else {
             // nothing with the same name exists

From 69be86ba074ccc8408d87185bf3baeb05bee5d8c Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 22:13:10 +0100
Subject: [PATCH 21/22] NodeItem: reduce complexity of addChild

---
 src/main/java/net/kemitix/node/NodeItem.java | 38 +++++++++++---------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index 979fddd..f9ebf47 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -132,28 +132,34 @@ class NodeItem<T> implements Node<T> {
      */
     @Override
     public void addChild(@NonNull final Node<T> child) {
-        if (this.equals(child) || isDescendantOf(child)) {
-            throw new NodeException("Child is an ancestor");
-        }
-        if (child.isNamed()) {
-            final Optional<Node<T>> existingChild = findChildByName(
-                    child.getName());
-            if (existingChild.isPresent() && existingChild.get() != child) {
-                throw new NodeException(
-                        "Node with that name already exists here");
-            }
-        }
+        verifyChildIsNotAnAncestor(child);
+        verifyChildWithSameNameDoesNotAlreadyExist(child);
         children.add(child);
         // update the child's parent if they don't have one or it is not this
-        Optional<Node<T>> childParent = child.getParent();
-        boolean isOrphan = !childParent.isPresent();
-        boolean hasDifferentParent = !isOrphan && !childParent.get()
-                .equals(this);
-        if (isOrphan || hasDifferentParent) {
+        val childParent = child.getParent();
+        if (!childParent.isPresent() || !childParent.get().equals(this)) {
             child.setParent(this);
         }
     }
 
+    private void verifyChildWithSameNameDoesNotAlreadyExist(
+            final @NonNull Node<T> child) {
+        if (child.isNamed()) {
+            findChildByName(child.getName())
+                    .filter(existingChild -> existingChild != child)
+                    .ifPresent(existingChild -> {
+                        throw new NodeException(
+                                "Node with that name already exists here");
+                    });
+        }
+    }
+
+    private void verifyChildIsNotAnAncestor(final @NonNull Node<T> child) {
+        if (this.equals(child) || isDescendantOf(child)) {
+            throw new NodeException("Child is an ancestor");
+        }
+    }
+
     /**
      * Creates a new node and adds it as a child of the current node.
      *

From 29a5ceca827430896bab8952961100d1c9dd88d5 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Sun, 4 Sep 2016 22:19:22 +0100
Subject: [PATCH 22/22] NodeItem: remove support for dynamic names

---
 src/main/java/net/kemitix/node/NodeItem.java | 26 --------------------
 1 file changed, 26 deletions(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index f9ebf47..12a7cef 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -8,7 +8,6 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.Function;
 
 /**
  * Represents a tree of nodes.
@@ -23,8 +22,6 @@ class NodeItem<T> implements Node<T> {
 
     private final Set<Node<T>> children = new HashSet<>();
 
-    private Function<Node<T>, String> nameSupplier;
-
     private Node<T> parent;
 
     private String name;
@@ -47,7 +44,6 @@ class NodeItem<T> implements Node<T> {
      */
     NodeItem(final T data) {
         this.data = data;
-        this.nameSupplier = (n) -> null;
     }
 
     /**
@@ -74,24 +70,8 @@ class NodeItem<T> implements Node<T> {
         setParent(parent);
     }
 
-    private String generateName() {
-        return getNameSupplier().apply(this);
-    }
-
-    private Function<Node<T>, String> getNameSupplier() {
-        if (nameSupplier != null) {
-            return nameSupplier;
-        }
-        // no test for parent as root nodes will always have a default name
-        // supplier
-        return ((NodeItem<T>) parent).getNameSupplier();
-    }
-
     @Override
     public String getName() {
-        if (name == null) {
-            return generateName();
-        }
         return name;
     }
 
@@ -380,14 +360,8 @@ class NodeItem<T> implements Node<T> {
     public void removeParent() {
         if (parent != null) {
             Node<T> oldParent = parent;
-            Function<Node<T>, String> supplier = getNameSupplier();
             parent = null;
             oldParent.removeChild(this);
-            if (this.nameSupplier == null) {
-                // this is now a root node, so must provide a default name
-                // supplier
-                this.nameSupplier = supplier;
-            }
         }
     }