diff --git a/checkstyle.xml b/checkstyle.xml deleted file mode 100644 index e54bdb6..0000000 --- a/checkstyle.xml +++ /dev/null @@ -1,192 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/pom.xml b/pom.xml index 3071a0d..a502cc3 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ net.kemitix kemitix-parent - 0.6.0 + 2.0.0 @@ -50,7 +50,6 @@ org.projectlombok lombok 1.16.8 - test junit diff --git a/src/main/java/net/kemitix/node/AbstractNodeItem.java b/src/main/java/net/kemitix/node/AbstractNodeItem.java index 6aa90e7..1fe6eab 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; @@ -9,9 +11,9 @@ import java.util.Set; * An abstract node item, providing default implementations for most read-only * operations. * - * @param the type of data stored in each node + * @author Paul Campbell * - * @author pcampbell + * @param the type of data stored in each node */ abstract class AbstractNodeItem implements Node { @@ -65,10 +67,7 @@ abstract class AbstractNodeItem implements Node { * @return an {@link Optional} containing the child node if found */ @Override - public Optional> findChild(final T child) { - if (child == null) { - throw new NullPointerException("child"); - } + public Optional> findChild(@NonNull final T child) { return children.stream().filter(node -> { final Optional d = node.getData(); return d.isPresent() && d.get().equals(child); @@ -102,27 +101,25 @@ abstract class AbstractNodeItem implements Node { * @return the child or null */ @Override - public Optional> findInPath(final List path) { - if (path == null) { - throw new NullPointerException("path"); + public Optional> findInPath(@NonNull final List path) { + if (path.isEmpty()) { + return Optional.empty(); } - if (path.size() > 0) { - Optional> found = findChild(path.get(0)); - if (found.isPresent()) { - if (path.size() > 1) { - return found.get().findInPath(path.subList(1, path.size())); - } - return found; + Node current = this; + for (T item : path) { + final Optional> child = current.findChild(item); + if (child.isPresent()) { + current = child.get(); + } else { + current = null; + break; } } - return Optional.empty(); + return Optional.ofNullable(current); } @Override - public Optional> findChildByName(final String named) { - if (named == null) { - throw new NullPointerException("name"); - } + public Optional> findChildByName(@NonNull final String named) { return children.stream() .filter(n -> n.getName().equals(named)) .findAny(); @@ -139,17 +136,18 @@ abstract class AbstractNodeItem implements Node { 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; 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.

* - * @param the type of data stored in each node + * @author Paul Campbell * - * @author pcampbell + * @param the type of data stored in each node */ final class ImmutableNodeItem extends AbstractNodeItem { diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index 7cda730..395ebb7 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 the type of data held in each node + * @author Paul Campbell * - * @author pcampbell + * @param the type of data held in each node */ public interface Node { @@ -113,6 +113,9 @@ public interface Node { * @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 findOrCreateChild(T child); diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index e688355..12a7cef 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -1,18 +1,20 @@ package net.kemitix.node; +import lombok.NonNull; +import lombok.val; + 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; /** * Represents a tree of nodes. * - * @param the type of data stored in each node + * @author Paul Campbell * - * @author pcampbell + * @param the type of data stored in each node */ class NodeItem implements Node { @@ -20,8 +22,6 @@ class NodeItem implements Node { private final Set> children = new HashSet<>(); - private Function, String> nameSupplier; - private Node parent; private String name; @@ -44,7 +44,6 @@ class NodeItem implements Node { */ NodeItem(final T data) { this.data = data; - this.nameSupplier = (n) -> null; } /** @@ -71,24 +70,8 @@ class NodeItem implements Node { setParent(parent); } - private String generateName() { - return getNameSupplier().apply(this); - } - - private Function, String> getNameSupplier() { - if (nameSupplier != null) { - return nameSupplier; - } - // no test for parent as root nodes will always have a default name - // supplier - return ((NodeItem) parent).getNameSupplier(); - } - @Override public String getName() { - if (name == null) { - return generateName(); - } return name; } @@ -128,32 +111,35 @@ class NodeItem implements Node { * @param child the node to add */ @Override - public void addChild(final Node child) { - if (child == null) { - throw new NullPointerException("child"); - } - if (this.equals(child) || isDescendantOf(child)) { - throw new NodeException("Child is an ancestor"); - } - if (child.isNamed()) { - final Optional> existingChild = findChildByName( - child.getName()); - if (existingChild.isPresent() && existingChild.get() != child) { - throw new NodeException( - "Node with that name already exists here"); - } - } + public void addChild(@NonNull final Node child) { + verifyChildIsNotAnAncestor(child); + verifyChildWithSameNameDoesNotAlreadyExist(child); children.add(child); // update the child's parent if they don't have one or it is not this - Optional> 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 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 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. * @@ -162,10 +148,7 @@ class NodeItem implements Node { * @return the new child node */ @Override - public Node createChild(final T child) { - if (child == null) { - throw new NullPointerException("child"); - } + public Node createChild(@NonNull final T child) { return new NodeItem<>(child, this); } @@ -184,10 +167,7 @@ class NodeItem implements Node { * @param descendants the line of descendants from the current node */ @Override - public void createDescendantLine(final List descendants) { - if (descendants == null) { - throw new NullPointerException("descendants"); - } + public void createDescendantLine(@NonNull final List descendants) { if (!descendants.isEmpty()) { findOrCreateChild(descendants.get(0)).createDescendantLine( descendants.subList(1, descendants.size())); @@ -201,13 +181,13 @@ class NodeItem implements Node { * @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 findOrCreateChild(final T child) { - if (child == null) { - throw new NullPointerException("child"); - } + public Node findOrCreateChild(@NonNull final T child) { return findChild(child).orElseGet(() -> createChild(child)); } @@ -219,10 +199,7 @@ class NodeItem implements Node { * @return an {@link Optional} containing the child node if found */ @Override - public Optional> findChild(final T child) { - if (child == null) { - throw new NullPointerException("child"); - } + public Optional> findChild(@NonNull final T child) { return children.stream().filter(node -> { final Optional d = node.getData(); return d.isPresent() && d.get().equals(child); @@ -257,10 +234,7 @@ class NodeItem implements Node { * @param parent the new parent node */ @Override - public final void setParent(final Node parent) { - if (parent == null) { - throw new NullPointerException("parent"); - } + public final void setParent(@NonNull final Node parent) { if (this.equals(parent) || parent.isDescendantOf(this)) { throw new NodeException("Parent is a descendant"); } @@ -279,64 +253,68 @@ class NodeItem implements Node { * @return the child or null */ @Override - public Optional> findInPath(final List path) { - if (path == null) { - throw new NullPointerException("path"); + public Optional> findInPath(@NonNull final List path) { + if (path.isEmpty()) { + return Optional.empty(); } - if (path.size() > 0) { - Optional> found = findChild(path.get(0)); - if (found.isPresent()) { - if (path.size() > 1) { - return found.get().findInPath(path.subList(1, path.size())); - } - return found; + Node current = this; + for (T item : path) { + final Optional> child = current.findChild(item); + if (child.isPresent()) { + current = child.get(); + } else { + current = null; + break; } } - return Optional.empty(); + return Optional.ofNullable(current); } @Override public void insertInPath(final Node nodeItem, final String... path) { if (path.length == 0) { - if (!nodeItem.isNamed()) { // nothing to conflict with - addChild(nodeItem); - return; - } - String nodeName = nodeItem.getName(); - final Optional> 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 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; - } - String item = path[0]; - final Optional> childNamed = findChildByName(item); - Node child; - if (!childNamed.isPresent()) { - child = new NodeItem<>(null, item, this); + insertChild(nodeItem); } else { - child = childNamed.get(); + 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 nodeItem) { + if (nodeItem.isNamed()) { + insertNamedChild(nodeItem); + } else { + // nothing to conflict with + addChild(nodeItem); + } + } + + private void insertNamedChild(final Node 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); } - child.insertInPath(nodeItem, Arrays.copyOfRange(path, 1, path.length)); } @Override - public Optional> findChildByName(final String named) { - if (named == null) { - throw new NullPointerException("name"); - } + public Optional> findChildByName(@NonNull final String named) { return children.stream() - .filter((Node t) -> t.getName().equals(named)) - .findAny(); + .filter((Node t) -> t.getName().equals(named)) + .findAny(); } @Override @@ -353,17 +331,18 @@ class NodeItem implements Node { 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))); + 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() { String currentName = getName(); @@ -381,14 +360,8 @@ class NodeItem implements Node { public void removeParent() { if (parent != null) { Node oldParent = parent; - Function, 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; - } } } 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/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 { +} 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");