Merge pull request #20 from kemitix/merge-nodeitem-into-abstract

Make ImmutableNodeItem an extension of NodeItem

src/main/java/net/kemitix/node/AbstractNodeItem.java

@@ -1,183 +0,0 @@
-/*
-The MIT License (MIT)
-
-Copyright (c) 2016 Paul Campbell
-
-Permission is hereby granted, free of charge, to any person obtaining a copy
-of this software and associated documentation files (the "Software"), to deal
-in the Software without restriction, including without limitation the rights
-to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
-copies of the Software, and to permit persons to whom the Software is
-furnished to do so, subject to the following conditions:
-
-The above copyright notice and this permission notice shall be included in all
-copies or substantial portions of the Software.
-
-THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
-OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
-SOFTWARE.
-*/
-
-package net.kemitix.node;
-
-import lombok.NonNull;
-
-import java.util.HashSet;
-import java.util.List;
-import java.util.Optional;
-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 (pcampbell@kemitix.net)
- */
-abstract class AbstractNodeItem<T> implements Node<T> {
-
-    private final Set<Node<T>> children;
-
-    private T data;
-
-    private String name;
-
-    private Node<T> parent;
-
-    /**
-     * Constructor.
-     *
-     * @param data     the data of the node
-     * @param name     the name of the node
-     * @param parent   the parent of the node, or null for a root node
-     * @param children the children of the node - must not be null
-     */
-    protected AbstractNodeItem(
-            final T data, final String name, final Node<T> parent, @NonNull final Set<Node<T>> children
-                              ) {
-        this.data = data;
-        this.name = name;
-        this.parent = parent;
-        this.children = children;
-    }
-
-    @Override
-    public String getName() {
-        return name;
-    }
-
-    @Override
-    public Optional<T> getData() {
-        return Optional.ofNullable(data);
-    }
-
-    @Override
-    public boolean isEmpty() {
-        return data == null;
-    }
-
-    @Override
-    public Optional<Node<T>> getParent() {
-        return Optional.ofNullable(parent);
-    }
-
-    @Override
-    public Set<Node<T>> getChildren() {
-        return new HashSet<>(children);
-    }
-
-    /**
-     * Fetches the node for the child if present.
-     *
-     * @param child the child's data to search for
-     *
-     * @return an {@link Optional} containing the child node if found
-     */
-    @Override
-    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);
-                       })
-                       .findAny();
-    }
-
-    @Override
-    public Node<T> getChild(final T child) {
-        return findChild(child).orElseThrow(() -> new NodeException("Child not found"));
-    }
-
-    /**
-     * Checks if the node is an ancestor.
-     *
-     * @param node the potential ancestor
-     *
-     * @return true if the node is an ancestor
-     */
-    @Override
-    public boolean isDescendantOf(final Node<T> node) {
-        return parent != null && (node.equals(parent) || parent.isDescendantOf(node));
-    }
-
-    /**
-     * Walks the node tree using the path to select each child.
-     *
-     * @param path the path to the desired child
-     *
-     * @return the child or null
-     */
-    @Override
-    public Optional<Node<T>> findInPath(@NonNull final List<T> path) {
-        if (path.isEmpty()) {
-            return Optional.empty();
-        }
-        Node<T> current = this;
-        for (int i = 0, pathSize = path.size(); i < pathSize && current != null; i++) {
-            current = current.findChild(path.get(i))
-                             .orElse(null);
-        }
-        return Optional.ofNullable(current);
-    }
-
-    @Override
-    public Optional<Node<T>> findChildByName(@NonNull final String named) {
-        return children.stream()
-                       .filter(n -> n.getName()
-                                     .equals(named))
-                       .findAny();
-    }
-
-    @Override
-    public Node<T> getChildByName(final String named) {
-        return findChildByName(named).orElseThrow(() -> new NodeException("Named child not found"));
-    }
-
-    @Override
-    public String drawTree(final int depth) {
-        final StringBuilder sb = new StringBuilder();
-        final String unnamed = "(unnamed)";
-        if (isNamed()) {
-            sb.append(formatByDepth(name, depth));
-        } else if (!children.isEmpty()) {
-            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;
-    }
-}

src/main/java/net/kemitix/node/ImmutableNodeItem.java

@@ -39,14 +39,15 @@ import java.util.Set;
  *
  * @author Paul Campbell (pcampbell@kemitix.net)
  */
-final class ImmutableNodeItem<T> extends AbstractNodeItem<T> {
+final class ImmutableNodeItem<T> extends NodeItem<T> {
 
     private static final String IMMUTABLE_OBJECT = "Immutable object";
 
     private ImmutableNodeItem(
             final T data, final String name, final Node<T> parent, final Set<Node<T>> children
                              ) {
-        super(data, name, parent, children);
+        super(data, name, null, children);
+        forceParent(parent);
     }
 
     /**
@@ -117,11 +118,6 @@ final class ImmutableNodeItem<T> extends AbstractNodeItem<T> {
         throw new UnsupportedOperationException(IMMUTABLE_OBJECT);
     }
 
-    @Override
-    public Node<T> findOrCreateChild(final T child) {
-        return findChild(child).orElseThrow(() -> new UnsupportedOperationException(IMMUTABLE_OBJECT));
-    }
-
     @Override
     public void insertInPath(final Node<T> node, final String... path) {
         throw new UnsupportedOperationException(IMMUTABLE_OBJECT);
@@ -136,5 +132,4 @@ final class ImmutableNodeItem<T> extends AbstractNodeItem<T> {
     public void removeParent() {
         throw new UnsupportedOperationException(IMMUTABLE_OBJECT);
     }
-
 }

src/main/java/net/kemitix/node/Node.java

@@ -129,19 +129,6 @@ public interface Node<T> {
      */
     void createDescendantLine(List<T> descendants);
 
-    /**
-     * Looks for a child node and returns it, creating a new child node if one
-     * isn't found.
-     *
-     * @param child the child's data to search or create with
-     *
-     * @return the found or created child node
-     *
-     * @deprecated use {@code node.findChild(child).orElseGet(() -> node.createChild(child))};
-     */
-    @Deprecated
-    Node<T> findOrCreateChild(T child);
-
     /**
      * Fetches the node for the child if present.
      *

src/main/java/net/kemitix/node/NodeItem.java

@@ -51,47 +51,31 @@ class NodeItem<T> implements Node<T> {
     private String name;
 
     /**
-     * Create named root node.
+     * Constructor.
      *
-     * @param data the data or null
+     * @param data     the data of the node
-     * @param name the name
+     * @param name     the name of the node
+     * @param parent   the parent of the node, or null for a root node
+     * @param children the children of the node - must not be null
      */
-    NodeItem(final T data, final String name) {
+    NodeItem(
-        this(data);
+            final T data, final String name, final Node<T> parent, @NonNull final Set<Node<T>> children
-        this.name = name;
+            ) {
-    }
-
-    /**
-     * Create unnamed root node.
-     *
-     * @param data the data or null
-     */
-    NodeItem(final T data) {
-        this.data = data;
-    }
-
-    /**
-     * Creates a node with a parent.
-     *
-     * @param data   the data or null
-     * @param parent the parent node
-     */
-    NodeItem(final T data, final Node<T> parent) {
-        this.data = data;
-        setParent(parent);
-    }
-
-    /**
-     * Creates a named node with a parent.
-     *
-     * @param data   the data or null
-     * @param name   the name
-     * @param parent the parent node
-     */
-    NodeItem(final T data, final String name, final Node<T> parent) {
         this.data = data;
         this.name = name;
-        setParent(parent);
+        if (parent != null) {
+            doSetParent(parent);
+        }
+        this.children.addAll(children);
+    }
+
+    /**
+     * Sets the parent of a node without updating the parent in the process as {@link #setParent(Node)} does.
+     *
+     * @param newParent The new parent node
+     */
+    protected void forceParent(final Node<T> newParent) {
+        this.parent = newParent;
     }
 
     @Override
@@ -130,16 +114,20 @@ class NodeItem<T> implements Node<T> {
      * @param parent the new parent node
      */
     @Override
-    public final void setParent(@NonNull final Node<T> parent) {
+    public void setParent(@NonNull final Node<T> parent) {
-        if (this.equals(parent) || parent.isDescendantOf(this)) {
+        doSetParent(parent);
+    }
+
+    private void doSetParent(@NonNull final Node<T> newParent) {
+        if (this.equals(newParent) || newParent.isDescendantOf(this)) {
             throw new NodeException("Parent is a descendant");
         }
         if (this.parent != null) {
             this.parent.getChildren()
                        .remove(this);
         }
-        this.parent = parent;
+        this.parent = newParent;
-        parent.addChild(this);
+        newParent.addChild(this);
     }
 
     @Override
@@ -191,7 +179,7 @@ class NodeItem<T> implements Node<T> {
      */
     @Override
     public Node<T> createChild(@NonNull final T child) {
-        return new NodeItem<>(child, this);
+        return new NodeItem<>(child, "", this, new HashSet<>());
     }
 
     @Override
@@ -211,26 +199,13 @@ class NodeItem<T> implements Node<T> {
     @Override
     public void createDescendantLine(@NonNull final List<T> descendants) {
         if (!descendants.isEmpty()) {
-            findOrCreateChild(descendants.get(0)).createDescendantLine(descendants.subList(1, descendants.size()));
+            val child = descendants.get(0);
+            val remainingLine = descendants.subList(1, descendants.size());
+            findChild(child).orElseGet(() -> createChild(child))
+                            .createDescendantLine(remainingLine);
         }
     }
 
-    /**
-     * Looks for a child node and returns it, creating a new child node if one
-     * isn't found.
-     *
-     * @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
-    public Node<T> findOrCreateChild(@NonNull final T child) {
-        return findChild(child).orElseGet(() -> createChild(child));
-    }
-
     /**
      * Fetches the node for the child if present.
      *
@@ -241,21 +216,14 @@ class NodeItem<T> implements Node<T> {
     @Override
     public Optional<Node<T>> findChild(@NonNull final T child) {
         return children.stream()
-                       .filter(node -> {
+                       .filter(node -> child.equals(node.getData()
-                           final Optional<T> d = node.getData();
+                                                        .orElseGet(() -> null)))
-                           return d.isPresent() && d.get()
+                       .findFirst();
-                                                    .equals(child);
-                       })
-                       .findAny();
     }
 
     @Override
     public Node<T> getChild(final T child) {
-        Optional<Node<T>> optional = findChild(child);
+        return findChild(child).orElseThrow(() -> new NodeException("Child not found"));
-        if (optional.isPresent()) {
-            return optional.get();
-        }
-        throw new NodeException("Child not found");
     }
 
     /**
@@ -295,9 +263,9 @@ class NodeItem<T> implements Node<T> {
         if (path.length == 0) {
             insertChild(nodeItem);
         } else {
-            val item = path[0];
+            val nextInPath = path[0];
-            findChildByName(item).orElseGet(() -> new NodeItem<>(null, item, this))
+            findChildByName(nextInPath).orElseGet(() -> new NodeItem<>(null, nextInPath, this, new HashSet<>()))
-                                 .insertInPath(nodeItem, Arrays.copyOfRange(path, 1, path.length));
+                                       .insertInPath(nodeItem, Arrays.copyOfRange(path, 1, path.length));
         }
     }
 

src/main/java/net/kemitix/node/Nodes.java

@@ -24,6 +24,7 @@ SOFTWARE.
 
 package net.kemitix.node;
 
+import java.util.HashSet;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -47,7 +48,7 @@ public final class Nodes {
      * @return the new node
      */
     public static <T> Node<T> unnamedRoot(final T data) {
-        return new NodeItem<>(data);
+        return new NodeItem<>(data, "", null, new HashSet<>());
     }
 
     /**
@@ -60,7 +61,7 @@ public final class Nodes {
      * @return the new node
      */
     public static <T> Node<T> namedRoot(final T data, final String name) {
-        return new NodeItem<>(data, name);
+        return new NodeItem<>(data, name, null, new HashSet<>());
     }
 
     /**
@@ -73,7 +74,7 @@ public final class Nodes {
      * @return the new node
      */
     public static <T> Node<T> unnamedChild(final T data, final Node<T> parent) {
-        return new NodeItem<>(data, parent);
+        return new NodeItem<>(data, "", parent, new HashSet<>());
     }
 
     /**
@@ -89,7 +90,7 @@ public final class Nodes {
     public static <T> Node<T> namedChild(
             final T data, final String name, final Node<T> parent
                                         ) {
-        return new NodeItem<>(data, name, parent);
+        return new NodeItem<>(data, name, parent, new HashSet<>());
     }
 
     /**

src/test/java/net/kemitix/node/ImmutableNodeItemTest.java

@@ -126,7 +126,7 @@ public class ImmutableNodeItemTest {
         immutableNode = Nodes.asImmutable(Nodes.unnamedRoot("subject"));
         expectImmutableException();
         //when
-        immutableNode.setParent(null);
+        immutableNode.setParent(Nodes.unnamedRoot("child"));
     }
 
     @Test
@@ -222,19 +222,6 @@ public class ImmutableNodeItemTest {
         }
     }
 
-    /**
-     * Test that if we pass null we get an exception.
-     */
-    @Test
-    public void findOrCreateChildShouldThrowNPEFWhenChildIsNull() {
-        //given
-        immutableNode = Nodes.asImmutable(Nodes.unnamedRoot("subject"));
-        exception.expect(NullPointerException.class);
-        exception.expectMessage("child");
-        //when
-        immutableNode.findOrCreateChild(null);
-    }
-
     /**
      * Test that we throw an exception when passed null.
      */
@@ -431,24 +418,4 @@ public class ImmutableNodeItemTest {
         //when
         immutableNode.insertInPath(null, "");
     }
-
-    @Test
-    public void findOrCreateChildShouldReturnChildWhenChildIsFound() {
-        //given
-        val root = Nodes.unnamedRoot("");
-        Nodes.namedChild("child", "child", root);
-        immutableNode = Nodes.asImmutable(root);
-        //when
-        val found = immutableNode.findOrCreateChild("child");
-        assertThat(found).extracting(Node::getName).contains("child");
-    }
-
-    @Test
-    public void findOrCreateChildShouldThrowExceptionWhenChildNotFound() {
-        //given
-        immutableNode = Nodes.asImmutable(Nodes.unnamedRoot(""));
-        expectImmutableException();
-        //when
-        immutableNode.findOrCreateChild("child");
-    }
 }

src/test/java/net/kemitix/node/NodeItemTest.java

@@ -10,6 +10,7 @@ import org.junit.rules.ExpectedException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Optional;
+import java.util.Set;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -84,6 +85,17 @@ public class NodeItemTest {
                                     .contains(parent);
     }
 
+    @Test
+    public void shouldAddAsChildWhenCreatedWithParent() {
+        //given
+        final Node<String> root = Nodes.namedRoot("root data", "root name");
+        //when
+        final Node<String> child = Nodes.namedChild("child data", "child name", root);
+        //then
+        final Set<Node<String>> children = root.getChildren();
+        assertThat(children).containsExactly(child);
+    }
+
     /**
      * Test that setting the parent on a node where the proposed parent is a
      * child of the node throws an exception.
@@ -457,52 +469,6 @@ public class NodeItemTest {
                         + "is created").isEmpty();
     }
 
-    /**
-     * Test that we can find a child of a node.
-     */
-    @Test
-    public void shouldFindExistingChildNode() {
-        //given
-        node = Nodes.unnamedRoot("subject");
-        val childData = "child";
-        val child = Nodes.unnamedChild(childData, node);
-        //when
-        val found = node.findOrCreateChild(childData);
-        //then
-        assertThat(found).as(
-                "when searching for a child by data, the matching child is "
-                        + "found").isSameAs(child);
-    }
-
-    /**
-     * Test that we create a missing child of a node.
-     */
-    @Test
-    public void shouldFindCreateNewChildNode() {
-        //given
-        node = Nodes.unnamedRoot("subject");
-        val childData = "child";
-        //when
-        val found = node.findOrCreateChild(childData);
-        //then
-        assertThat(found.getData()).as(
-                "when searching for a non-existent child by data, a new node "
-                        + "is created").contains(childData);
-    }
-
-    /**
-     * Test that if we pass null we get an exception.
-     */
-    @Test
-    public void findOrCreateChildShouldThrowNPEFWhenChildIsNull() {
-        //given
-        node = Nodes.unnamedRoot("subject");
-        exception.expect(NullPointerException.class);
-        exception.expectMessage("child");
-        //when
-        node.findOrCreateChild(null);
-    }
-
     /**
      * Test that we can get the node for a child.
      */

src/test/java/net/kemitix/node/NodesTest.java

@@ -24,7 +24,7 @@ public class NodesTest {
         val node = Nodes.unnamedRoot("data");
         SoftAssertions softly = new SoftAssertions();
         softly.assertThat(node.getData()).contains("data");
-        softly.assertThat(node.getName()).isNull();
+        softly.assertThat(node.getName()).isEmpty();
         softly.assertAll();
     }
 
@@ -43,7 +43,7 @@ public class NodesTest {
         val node = Nodes.unnamedChild("data", parent);
         SoftAssertions softly = new SoftAssertions();
         softly.assertThat(node.getData()).contains("data");
-        softly.assertThat(node.getName()).isNull();
+        softly.assertThat(node.getName()).isEmpty();
         softly.assertThat(node.getParent()).contains(parent);
         softly.assertAll();
     }