From 921cf98b13550c9255a071f17df5c9c58bb35a67 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Mon, 23 May 2016 11:57:50 +0100 Subject: [PATCH 01/13] Add assertj-core dependency for testing --- node.iml | 1 + pom.xml | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/node.iml b/node.iml index a66a2bf..b2d3a3d 100644 --- a/node.iml +++ b/node.iml @@ -14,5 +14,6 @@ + \ No newline at end of file diff --git a/pom.xml b/pom.xml index 32577de..c488538 100644 --- a/pom.xml +++ b/pom.xml @@ -14,6 +14,10 @@ 0.6.0 + + 3.4.1 + + https://github.com/kemitix/node/issues GitHub Issues @@ -48,5 +52,11 @@ 1.3 test + + org.assertj + assertj-core + ${assertj.version} + test + From 36efe5d83ab5e3f63c6cf3cd208b54a19c1600ce Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 09:57:11 +0100 Subject: [PATCH 02/13] Node: may be empty, having no data * isEmpty() --- src/main/java/net/kemitix/node/Node.java | 7 +++++++ src/main/java/net/kemitix/node/NodeItem.java | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index 002bead..2d9c67e 100644 --- a/src/main/java/net/kemitix/node/Node.java +++ b/src/main/java/net/kemitix/node/Node.java @@ -20,6 +20,13 @@ public interface Node { */ T getData(); + /** + * Returns true if the node is empty (has no data). + * + * @return true is data is null + */ + boolean isEmpty(); + /** * Fetch the parent node. *

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index d11ff23..c3a3b45 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -50,6 +50,11 @@ public class NodeItem implements Node { return data; } + @Override + public boolean isEmpty() { + return data == null; + } + @Override public Node getParent() { return parent; From b18020708bb21a1e44bd0de0bda57739b2901c6b Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 10:59:16 +0100 Subject: [PATCH 03/13] Node: may have names and a functional name supplier Names, where present, must be unique for each parent. Node: * String getName() * void setName(String name) * boolean isNamed() NodeItem - replaces all constructor: * (data) * (data, name) * (data, nameSupplier) * (data, parent) * (data, nameSupplier, parent) The name supplier takes a node and generates a string at the time the node is constructed. The root node has a default name supplier that returns null which means that the node is considered unnamed. Other nodes may provide their own name supplier that would be used be new nodes created within their subtree. --- src/main/java/net/kemitix/node/Node.java | 21 ++++ src/main/java/net/kemitix/node/NodeItem.java | 111 +++++++++++++++++-- 2 files changed, 121 insertions(+), 11 deletions(-) diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index 2d9c67e..66f9910 100644 --- a/src/main/java/net/kemitix/node/Node.java +++ b/src/main/java/net/kemitix/node/Node.java @@ -13,6 +13,20 @@ import java.util.Set; */ public interface Node { + /** + * Fetch the name of the node. + * + * @return the name of the node + */ + String getName(); + + /** + * Sets the explicit name for a node. + * + * @param name the new name + */ + void setName(String name); + /** * Fetch the data held within the node. * @@ -112,4 +126,11 @@ public interface Node { */ Optional> walkTree(final List path); + /** + * Returns true if the Node has a name. + * + * @return true if the node has a name + */ + boolean isNamed(); + } diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index c3a3b45..6815350 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -4,6 +4,7 @@ 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. @@ -16,33 +17,108 @@ public class NodeItem implements Node { private final T data; - private Node parent; - private final Set> children = new HashSet<>(); + private Function, String> nameSupplier; + + private Node parent; + + private String name; + /** - * Creates a root node. + * Create unnamed root node. * - * @param data the value of the node + * @param data the data or null */ public NodeItem(final T data) { - this(data, null); + this.data = data; + this.nameSupplier = (n) -> null; + } + + /** + * Create named root node. + * + * @param data the data or null + * @param name the name + */ + public NodeItem(final T data, final String name) { + this(data); + this.name = name; + } + + /** + * Creates root node with a name supplier. + * + * @param data the data or null + * @param nameSupplier the name supplier function + */ + public NodeItem( + final T data, final Function, String> nameSupplier) { + this(data); + this.nameSupplier = nameSupplier; + name = generateName(); } /** * Creates a node with a parent. * - * @param data the value of the node + * @param data the data or null * @param parent the parent node */ public NodeItem(final T data, final Node parent) { - if (data == null) { - throw new NullPointerException("data"); - } this.data = data; - if (parent != null) { - setParent(parent); + setParent(parent); + this.name = generateName(); + } + + /** + * Creates a named node with a parent. + * + * @param data the data or null + * @param name the name + * @param parent the parent node + */ + public NodeItem(final T data, final String name, final Node parent) { + this.data = data; + this.name = name; + setParent(parent); + } + + /** + * Creates a node with a name supplier and a parent. + * + * @param data the data or null + * @param nameSupplier the name supplier function + * @param parent the parent node + */ + public NodeItem( + final T data, final Function, String> nameSupplier, + final Node parent) { + this(data, nameSupplier); + 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() { + return name; + } + + @Override + public void setName(final String name) { + this.name = name; } @Override @@ -98,6 +174,14 @@ public class NodeItem implements Node { if (this.equals(child) || isChildOf(child)) { throw new NodeException("Child is an ancestor"); } + if (child.isNamed()) { + final Optional> existingChild = findChildNamed( + child.getName()); + if (existingChild.isPresent() && existingChild.get() != child) { + throw new NodeException( + "Node with that name already exists here"); + } + } children.add(child); if (child.getParent() == null || !child.getParent().equals(this)) { child.setParent(this); @@ -202,6 +286,11 @@ public class NodeItem implements Node { public Node createChild(final T child) { if (child == null) { throw new NullPointerException("child"); + @Override + public boolean isNamed() { + return name != null && name.length() > 0; + } + } return new NodeItem<>(child, this); } From b278fc0f988725d2c09b1df5ee783a9b026748ae Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 11:03:19 +0100 Subject: [PATCH 04/13] Node: add {get,find}ChildNamed() methods Both methods look for a child with the given name as an immediate child of the current node. findChildNamed: Will return an Optional containing the found node or empty. getChildNames: Is more insistent and will return the found node itself. If a node by that name is not found, then a NodeException will be thrown. --- src/main/java/net/kemitix/node/Node.java | 19 +++++++++++++++++++ src/main/java/net/kemitix/node/NodeItem.java | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index 66f9910..c569773 100644 --- a/src/main/java/net/kemitix/node/Node.java +++ b/src/main/java/net/kemitix/node/Node.java @@ -127,6 +127,25 @@ public interface Node { Optional> walkTree(final List path); /** + /** + * Searches for a child with the name given. + * + * @param name the name of the child + * + * @return an Optional containing the child found or empty + */ + Optional> findChildNamed(String name); + + /** + * Returns the child with the given name. If one can't be found a + * NodeException is thrown. + * + * @param name the name of the child + * + * @return the node + */ + Node getChildNamed(String name); + * Returns true if the Node has a name. * * @return true if the node has a name diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index 6815350..fb8908b 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -286,6 +286,25 @@ public class NodeItem implements Node { public Node createChild(final T child) { if (child == null) { throw new NullPointerException("child"); + @Override + public Optional> findChildNamed(final String named) { + if (named == null) { + throw new NullPointerException("name"); + } + return children.stream() + .filter((Node t) -> t.getName().equals(named)) + .findAny(); + } + + @Override + public Node getChildNamed(final String named) { + final Optional> optional = findChildNamed(named); + if (optional.isPresent()) { + return optional.get(); + } + throw new NodeException("Named child not found"); + } + @Override public boolean isNamed() { return name != null && name.length() > 0; From b2c3032ec06ccd0df5d7062c2583bd8ddb4e2291 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 11:25:59 +0100 Subject: [PATCH 05/13] Node:drawTree(): creates a String representing the node in the tree The NodeItem implementation on includes nodes with names, or where they have child nodes. In which case the are shown as '(unnamed)'. --- src/main/java/net/kemitix/node/Node.java | 10 ++++++++++ src/main/java/net/kemitix/node/NodeItem.java | 15 +++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index c569773..0a96c66 100644 --- a/src/main/java/net/kemitix/node/Node.java +++ b/src/main/java/net/kemitix/node/Node.java @@ -146,6 +146,16 @@ public interface Node { */ Node getChildNamed(String name); + /** + * Draw a representation of the tree. + * + * @param depth current depth for recursion + * + * @return a representation of the tree + */ + String drawTree(int depth); + + /** * Returns true if the Node has a name. * * @return true if the node has a name diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index fb8908b..88764e4 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -305,6 +305,21 @@ public class NodeItem implements Node { throw 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(String.format("[%1$" + (depth + name.length()) + "s]\n", + name)); + } else if (!children.isEmpty()) { + sb.append(String.format("[%1$" + (depth + unnamed) + "s]\n", + unnamed)); + } + getChildren().stream().forEach(c -> sb.append(c.drawTree(depth + 1))); + return sb.toString(); + } + @Override public boolean isNamed() { return name != null && name.length() > 0; From 9fca56b4c6781fd1b2cd2041c241d2630ce3bba3 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 11:30:55 +0100 Subject: [PATCH 06/13] Node: remove{Child,Parent}(): split trees branches apart removeChild(node): removes the node from children of the current node, making the child node into a new root node. removeParent(): removes the current from from it's parent's children, making itself into a new root node. --- src/main/java/net/kemitix/node/Node.java | 11 +++++++++++ src/main/java/net/kemitix/node/NodeItem.java | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index 0a96c66..4d95501 100644 --- a/src/main/java/net/kemitix/node/Node.java +++ b/src/main/java/net/kemitix/node/Node.java @@ -162,4 +162,15 @@ public interface Node { */ boolean isNamed(); + /** + * Remove the node from the children. + * + * @param node the node to be removed + */ + void removeChild(Node node); + + /** + * Removes the parent from the node. Makes the node into a new root node. + */ + void removeParent(); } diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index 88764e4..c02cf86 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -305,6 +305,26 @@ public class NodeItem implements Node { throw new NodeException("Named child not found"); } + @Override + public void removeChild(final Node node) { + if (children.remove(node)) { + node.removeParent(); + } + } + + @Override + public void removeParent() { + if (parent != null) { + parent.removeChild(this); + parent = null; + if (nameSupplier == null) { + // this is now a root node, so must provide a default name + // supplier + nameSupplier = n -> null; + } + } + } + @Override public String drawTree(final int depth) { final StringBuilder sb = new StringBuilder(); From 45bd77bbcad81d1455c02ec5b77937d8e1ba15ea Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 11:35:33 +0100 Subject: [PATCH 07/13] Node.placeNodeIn(): add node to tree under the path of named elements Any intervening nodes that don't exist will be created. e.g. placeNodeIn(node, "alpha", "beta") Will add node: [root] "alpha" "beta" node --- src/main/java/net/kemitix/node/Node.java | 8 ++++ src/main/java/net/kemitix/node/NodeItem.java | 41 ++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index 4d95501..5ce9d56 100644 --- a/src/main/java/net/kemitix/node/Node.java +++ b/src/main/java/net/kemitix/node/Node.java @@ -127,6 +127,14 @@ public interface Node { Optional> walkTree(final List path); /** + * Places the node in the tree under by the path. Intervening empty + * nodes are created as needed. + * + * @param node the node to place + * @param path the path to contain the new node + */ + void placeNodeIn(Node node, String... path); + /** * Searches for a child with the name given. * diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index c02cf86..f338e9a 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -1,5 +1,6 @@ package net.kemitix.node; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -325,6 +326,46 @@ public class NodeItem implements Node { } } + @Override + public void placeNodeIn(final Node nodeItem, final String... path) { + if (path.length == 0) { + if (nodeItem.isNamed()) { + final Optional> childNamed = findChildNamed( + nodeItem.getName()); + if (childNamed.isPresent()) { + final Node existing = childNamed.get(); + if (!existing.isEmpty()) { + throw new NodeException( + "A non-empty node with that name already " + + "exists here"); + } else { + existing.getChildren().forEach(nodeItem::addChild); + existing.removeParent(); + nodeItem.setParent(this); + return; + } + } + } + addChild(nodeItem); + return; + } + String item = path[0]; + final Optional> childNamed = findChildNamed(item); + Node child; + if (!childNamed.isPresent()) { + child = new NodeItem<>(null, item, this); + } else { + child = childNamed.get(); + if (child.isEmpty()) { + if (path.length == 1) { + getChildren().forEach(nodeItem::addChild); + nodeItem.setParent(this); + } + } + } + child.placeNodeIn(nodeItem, Arrays.copyOfRange(path, 1, path.length)); + } + @Override public String drawTree(final int depth) { final StringBuilder sb = new StringBuilder(); From b456e183161caba29c015abd69a65de81fdc9463 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 11:53:13 +0100 Subject: [PATCH 08/13] NodeItem: fix up --- src/main/java/net/kemitix/node/NodeItem.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index f338e9a..bc87179 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -287,6 +287,10 @@ public class NodeItem implements Node { public Node createChild(final T child) { if (child == null) { throw new NullPointerException("child"); + } + return new NodeItem<>(child, this); + } + @Override public Optional> findChildNamed(final String named) { if (named == null) { @@ -386,8 +390,4 @@ public class NodeItem implements Node { return name != null && name.length() > 0; } - } - return new NodeItem<>(child, this); - } - } From 180f325f798b88343fa9ccfcb778af8f0f3ee674 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 11:53:52 +0100 Subject: [PATCH 09/13] NodeItemTest: overhaul of test suite --- .../java/net/kemitix/node/NodeItemTest.java | 493 +++++++++++++----- 1 file changed, 350 insertions(+), 143 deletions(-) diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java index e22605f..623ea94 100644 --- a/src/test/java/net/kemitix/node/NodeItemTest.java +++ b/src/test/java/net/kemitix/node/NodeItemTest.java @@ -1,13 +1,15 @@ package net.kemitix.node; import lombok.val; -import org.junit.Assert; +import org.assertj.core.api.SoftAssertions; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; -import static org.hamcrest.CoreMatchers.hasItem; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.nullValue; +import static org.assertj.core.api.Assertions.assertThat; +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; import java.util.Arrays; import java.util.Collections; import java.util.Optional; @@ -19,32 +21,51 @@ import java.util.Optional; */ public class NodeItemTest { - /** - * Class under test. - */ + @Rule + public ExpectedException exception = ExpectedException.none(); + private Node node; - /** - * Test that node data is recoverable. - */ @Test - public void shouldReturnNodeData() { + public void getDataReturnsData() { //given val data = "this node data"; //when node = new NodeItem<>(data); //then - Assert.assertThat("can get the data from a node", node.getData(), - is(data)); + assertThat(node.getData()).as("can get the data from a node"). + isSameAs(data); } - /** - * Test that passing null as node data throws exception. - */ - @Test(expected = NullPointerException.class) - public void shouldThrowNPEWhenDataIsNull() { + @Test + public void canCreateAnEmptyAndUnnamedNode() { //when node = new NodeItem<>(null); + //then + SoftAssertions softly = new SoftAssertions(); + softly.assertThat(node.isEmpty()).as("node is empty").isTrue(); + softly.assertThat(node.isNamed()).as("node is unnamed").isFalse(); + softly.assertAll(); + } + + @Test + public void canCreateNodeWithParentAndCustomNameSupplier() { + //given + node = new NodeItem<>(null, n -> "root name supplier"); + //when + val child = new NodeItem<>(null, n -> "overridden", node); + //then + assertThat(child.getName()).isEqualTo("overridden"); + } + + @Test + public void canSetName() { + //given + node = new NodeItem<>(null); + //when + node.setName("named"); + //then + assertThat(node.getName()).isEqualTo("named"); } /** @@ -53,10 +74,10 @@ public class NodeItemTest { @Test public void shouldHaveNullForDefaultParent() { //given - node = new NodeItem<>("data"); + node = new NodeItem<>("data", Node::getData); //then - Assert.assertThat("node created without a parent has null as parent", - node.getParent(), nullValue()); + assertThat(node.getParent()).as( + "node created without a parent has null as parent").isNull(); } /** @@ -65,23 +86,26 @@ public class NodeItemTest { @Test public void shouldReturnNodeParent() { //given - val parent = new NodeItem("parent"); + val parent = new NodeItem("parent", Node::getData); //when node = new NodeItem<>("subject", parent); //then - Assert.assertThat("node created with a parent can return the parent", - node.getParent(), is(parent)); + assertThat(node.getParent()).as( + "node created with a parent can return the parent") + .isSameAs(parent); } /** * Test that setting the parent on a node where the proposed parent is a * child of the node throws an exception. */ - @Test(expected = NodeException.class) - public void shouldThrowNEWhenSettingParentToAChild() { + @Test + public void setParentShouldThrowNodeExceptionWhenParentIsAChild() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); val child = new NodeItem("child", node); + exception.expect(NodeException.class); + exception.expectMessage("Parent is a descendant"); //when node.setParent(child); } @@ -91,16 +115,16 @@ public class NodeItemTest { * child of the parent. */ @Test + @SuppressWarnings("unchecked") public void shouldAddNewNodeAsChildToParent() { //given - val parent = new NodeItem("parent"); + val parent = new NodeItem("parent", Node::getData); //when node = new NodeItem<>("subject", parent); //then - Assert.assertThat( + assertThat(parent.getChildren()).as( "when a node is created with a parent, the parent has the new" - + " node among it's children", parent.getChildren(), - hasItem(node)); + + " node among it's children").contains(node); } /** @@ -109,23 +133,25 @@ public class NodeItemTest { @Test public void shouldReturnSetParent() { //given - node = new NodeItem<>("subject"); - val parent = new NodeItem("parent"); + node = new NodeItem<>("subject", Node::getData); + val parent = new NodeItem("parent", Node::getData); //when node.setParent(parent); //then - Assert.assertThat( + assertThat(node.getParent()).as( "when a node is assigned a new parent that parent can be " - + "returned", node.getParent(), is(parent)); + + "returned").isSameAs(parent); } /** * Test that we throw an exception when passed null. */ - @Test(expected = NullPointerException.class) + @Test public void shouldThrowNPEWhenSetParentNull() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); + exception.expect(NullPointerException.class); + exception.expectMessage("parent"); //when node.setParent(null); } @@ -134,10 +160,12 @@ public class NodeItemTest { * Test that we throw an exceptions when attempting to node as its own * parent. */ - @Test(expected = NodeException.class) - public void shouldThrowNEWhenSetParentSelf() { + @Test + public void setParentShouldThrowNodeExceptionWhenParentIsSelf() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); + exception.expect(NodeException.class); + exception.expectMessage("Parent is a descendant"); //when node.setParent(node); } @@ -149,19 +177,18 @@ public class NodeItemTest { @Test public void shouldUpdateOldParentWhenNodeSetToNewParent() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); val child = node.createChild("child"); - val newParent = new NodeItem("newParent"); + val newParent = new NodeItem("newParent", Node::getData); //when child.setParent(newParent); //then - Assert.assertThat( + assertThat(child.getParent()).as( "when a node is assigned a new parent, the old parent is " - + "replaced", child.getParent(), is(newParent)); - Assert.assertThat( + + "replaced").isSameAs(newParent); + assertThat(node.getChild("child").isPresent()).as( "when a node is assigned a new parent, the old parent no " - + "longer has the node among it's children", - node.getChild("child").isPresent(), is(false)); + + "longer has the node among it's children").isFalse(); } /** @@ -171,30 +198,31 @@ public class NodeItemTest { @Test public void shouldRemoveNodeFromOldParentWhenAddedAsChildToNewParent() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); val child = node.createChild("child"); - val newParent = new NodeItem("newParent"); + val newParent = new NodeItem("newParent", Node::getData); //when newParent.addChild(child); //then - Assert.assertThat( + assertThat(child.getParent()).as( "when a node with an existing parent is added as a child " - + "to another node, then the old parent is replaced", - child.getParent(), is(newParent)); - Assert.assertThat( + + "to another node, then the old parent is replaced") + .isSameAs(newParent); + assertThat(node.getChild("child").isPresent()).as( "when a node with an existing parent is added as a child to " + "another node, then the old parent no longer has " - + "the node among it's children", - node.getChild("child").isPresent(), is(false)); + + "the node among it's children").isFalse(); } /** * Test that adding null as a child throws an exception. */ - @Test(expected = NullPointerException.class) + @Test public void shouldThrowNPEWhenAddingNullAsChild() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); + exception.expect(NullPointerException.class); + exception.expectMessage("child"); //when node.addChild(null); } @@ -203,25 +231,28 @@ public class NodeItemTest { * Test that adding a child is returned. */ @Test + @SuppressWarnings("unchecked") public void shouldReturnAddedChild() { //given - node = new NodeItem<>("subject"); - val child = new NodeItem("child"); + node = new NodeItem<>("subject", Node::getData); + val child = new NodeItem("child", Node::getData); //when node.addChild(child); //then - Assert.assertThat( + assertThat(node.getChildren()).as( "when a node is added as a child, the node is among the " - + "children", node.getChildren(), hasItem(child)); + + "children").contains(child); } /** * Test that adding a node as it's own child throws an exception. */ - @Test(expected = NodeException.class) - public void shouldThrowNEWhenAddingANodeAsOwnChild() { + @Test + public void addChildShouldThrowNodeExceptionWhenAddingANodeAsOwnChild() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); + exception.expect(NodeException.class); + exception.expectMessage("Child is an ancestor"); //then node.addChild(node); } @@ -229,10 +260,12 @@ public class NodeItemTest { /** * Test that adding a node to itself as a child causes an exception. */ - @Test(expected = NodeException.class) - public void shouldThrowWhenAddingSelfAsChild() { + @Test + public void addChildShouldThrowNodeExceptionWhenAddingSelfAsChild() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); + exception.expect(NodeException.class); + exception.expectMessage("Child is an ancestor"); //when node.addChild(node); } @@ -241,11 +274,13 @@ public class NodeItemTest { * Test that adding the parent of a node to the node as a child causes an * exception. */ - @Test(expected = NodeException.class) - public void shouldThrowWhenAddingParentAsChild() { + @Test + public void addChildShouldThrowNodeExceptionWhenChildIsParent() { //given - val parent = new NodeItem("parent"); + val parent = new NodeItem("parent", Node::getData); node = new NodeItem<>("subject", parent); + exception.expect(NodeException.class); + exception.expectMessage("Child is an ancestor"); //when node.addChild(parent); } @@ -254,12 +289,14 @@ public class NodeItemTest { * Test that adding the grandparent to a node as a child causes an * exception. */ - @Test(expected = NodeException.class) - public void shouldThrowWhenAddingGrandParentAsChild() { + @Test + public void addChildShouldThrowNodeExceptionWhenAddingGrandParentAsChild() { //given - val grandParent = new NodeItem("grandparent"); + val grandParent = new NodeItem("grandparent", Node::getData); val parent = new NodeItem("parent", grandParent); node = new NodeItem<>("subject", parent); + exception.expect(NodeException.class); + exception.expectMessage("Child is an ancestor"); //when node.addChild(grandParent); } @@ -270,14 +307,14 @@ public class NodeItemTest { @Test public void shouldSetParentOnChildWhenAddedAsChild() { //given - val child = new NodeItem("child"); - node = new NodeItem<>("subject"); + val child = new NodeItem("child", Node::getData); + node = new NodeItem<>("subject", Node::getData); //when node.addChild(child); //then - Assert.assertThat( + assertThat(child.getParent()).as( "when a node is added as a child, the child has the node as " - + "its parent", child.getParent(), is(node)); + + "its parent").isSameAs(node); } /** @@ -287,7 +324,7 @@ public class NodeItemTest { public void shouldWalkTreeToNode() { //given val grandparent = "grandparent"; - val grandParentNode = new NodeItem(grandparent); + val grandParentNode = new NodeItem(grandparent, Node::getData); val parent = "parent"; val parentNode = new NodeItem(parent, grandParentNode); val subject = "subject"; @@ -295,12 +332,12 @@ public class NodeItemTest { //when val result = grandParentNode.walkTree(Arrays.asList(parent, subject)); //then - Assert.assertThat("when we walk the tree to a node it is found", - result.isPresent(), is(true)); + assertThat(result.isPresent()).as( + "when we walk the tree to a node it is found").isTrue(); if (result.isPresent()) { - Assert.assertThat( - "when we walk the tree to a node the correct node is found", - result.get(), is(node)); + assertThat(result.get()).as( + "when we walk the tree to a node the correct node is found") + .isSameAs(node); } } @@ -312,24 +349,26 @@ public class NodeItemTest { public void shouldNotFindNonExistentChildNode() { //given val parent = "parent"; - val parentNode = new NodeItem(parent); + val parentNode = new NodeItem(parent, Node::getData); val subject = "subject"; node = new NodeItem<>(subject, parentNode); //when val result = parentNode.walkTree(Arrays.asList(subject, "no child")); //then - Assert.assertThat( + assertThat(result.isPresent()).as( "when we walk the tree to a node that doesn't exists, nothing" - + " is found", result.isPresent(), is(false)); + + " is found").isFalse(); } /** * Test that when we pass null we get an exception. */ - @Test(expected = NullPointerException.class) + @Test public void shouldThrowNEWhenWalkTreeNull() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); + exception.expect(NullPointerException.class); + exception.expectMessage("path"); //when node.walkTree(null); } @@ -341,9 +380,11 @@ public class NodeItemTest { @Test public void shouldReturnEmptyForEmptyWalkTreePath() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); //when - node.walkTree(Collections.emptyList()); + val result = node.walkTree(Collections.emptyList()); + //then + assertThat(result).isEmpty(); } /** @@ -352,7 +393,7 @@ public class NodeItemTest { @Test public void shouldCreateDescendantNodes() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); val alphaData = "alpha"; val betaData = "beta"; val gammaData = "gamma"; @@ -361,36 +402,34 @@ public class NodeItemTest { Arrays.asList(alphaData, betaData, gammaData)); //then val alphaOptional = node.getChild(alphaData); - Assert.assertThat( - "when creating a descendant line, the first element is found", - alphaOptional.isPresent(), is(true)); + assertThat(alphaOptional.isPresent()).as( + "when creating a descendant line, the first element is found") + .isTrue(); if (alphaOptional.isPresent()) { val alpha = alphaOptional.get(); - Assert.assertThat( + assertThat(alpha.getParent()).as( "when creating a descendant line, the first element has " - + "the current node as its parent", - alpha.getParent(), is(node)); + + "the current node as its parent").isSameAs(node); val betaOptional = alpha.getChild(betaData); - Assert.assertThat( + assertThat(betaOptional.isPresent()).as( "when creating a descendant line, the second element is " - + "found", betaOptional.isPresent(), is(true)); + + "found").isTrue(); if (betaOptional.isPresent()) { val beta = betaOptional.get(); - Assert.assertThat( + assertThat(beta.getParent()).as( "when creating a descendant line, the second element " - + "has the first as its parent", - beta.getParent(), is(alpha)); + + "has the first as its parent") + .isSameAs(alpha); val gammaOptional = beta.getChild(gammaData); - Assert.assertThat( + assertThat(gammaOptional.isPresent()).as( "when creating a descendant line, the third element " - + "is found", gammaOptional.isPresent(), - is(true)); + + "is found").isTrue(); if (gammaOptional.isPresent()) { val gamma = gammaOptional.get(); - Assert.assertThat( + assertThat(gamma.getParent()).as( "when creating a descendant line, the third " - + "element has the second as its parent", - gamma.getParent(), is(beta)); + + "element has the second as its parent") + .isSameAs(beta); } } } @@ -400,10 +439,12 @@ public class NodeItemTest { * Test that if we pass null to create a chain of descendant nodes we get an * exception. */ - @Test(expected = NullPointerException.class) - public void shouldThrowNPEWhenCreateDescendantNull() { + @Test + public void createDescendantLineShouldThrowNPEWhenDescendantsAreNull() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); + exception.expect(NullPointerException.class); + exception.expectMessage("descendants"); //when node.createDescendantLine(null); } @@ -414,13 +455,13 @@ public class NodeItemTest { @Test public void shouldChangeNothingWhenCreateDescendantEmpty() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); //when node.createDescendantLine(Collections.emptyList()); //then - Assert.assertThat( + assertThat(node.getChildren()).as( "when creating a descendant line from an empty list, nothing " - + "is created", node.getChildren().size(), is(0)); + + "is created").isEmpty(); } /** @@ -429,15 +470,15 @@ public class NodeItemTest { @Test public void shouldFindExistingChildNode() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); val childData = "child"; val child = new NodeItem(childData, node); //when val found = node.findOrCreateChild(childData); //then - Assert.assertThat( + assertThat(found).as( "when searching for a child by data, the matching child is " - + "found", found, is(child)); + + "found").isSameAs(child); } /** @@ -446,23 +487,25 @@ public class NodeItemTest { @Test public void shouldFindCreateNewChildNode() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); val childData = "child"; //when val found = node.findOrCreateChild(childData); //then - Assert.assertThat( - "when searching for a child by data, a new node is created", - found.getData(), is(childData)); + assertThat(found.getData()).as( + "when searching for a child by data, a new node is created") + .isSameAs(childData); } /** * Test that if we pass null we get an exception. */ - @Test(expected = NullPointerException.class) - public void shouldThrowNPEFWhenFindOrCreateChildNull() { + @Test + public void findOrCreateChildShouldThrowNPEFWhenChildIsNull() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); + exception.expect(NullPointerException.class); + exception.expectMessage("child"); //when node.findOrCreateChild(null); } @@ -473,29 +516,31 @@ public class NodeItemTest { @Test public void shouldGetChild() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); val childData = "child"; - val child = new NodeItem(childData); + val child = new NodeItem(childData, Node::getData); node.addChild(child); //when val found = node.getChild(childData); //then - Assert.assertThat("when retrieving a child by its data, it is found", - found.isPresent(), is(true)); + assertThat(found.isPresent()).as( + "when retrieving a child by its data, it is found").isTrue(); if (found.isPresent()) { - Assert.assertThat( + assertThat(found.get()).as( "when retrieving a child by its data, it is the expected " - + "node", found.get(), is(child)); + + "node").isSameAs(child); } } /** * Test that we throw an exception when passed null. */ - @Test(expected = NullPointerException.class) - public void shouldThrowNPEWhenGetChildNull() { + @Test + public void getChildShouldThrowNPEWhenThereIsNoChild() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("data", Node::getData); + exception.expect(NullPointerException.class); + exception.expectMessage("child"); //when node.getChild(null); } @@ -507,34 +552,196 @@ public class NodeItemTest { @Test public void shouldCreateChild() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); val childData = "child"; //when val child = node.createChild(childData); //then - Assert.assertThat( + assertThat(child.getParent()).as( "when creating a child node, the child has the current node " - + "as its parent", child.getParent(), is(node)); + + "as its parent").isSameAs(node); val foundChild = node.getChild(childData); - Assert.assertThat( + assertThat(foundChild.isPresent()).as( "when creating a child node, the child can be found by its " - + "data", foundChild.isPresent(), is(true)); + + "data").isTrue(); if (foundChild.isPresent()) { - Assert.assertThat( + assertThat(foundChild.get()).as( "when creating a child node, the correct child can be " - + "found by its data", foundChild.get(), is(child)); + + "found by its data").isSameAs(child); } } /** * Test that we throw an exception when passed null. */ - @Test(expected = NullPointerException.class) - public void shouldThrowNPEWhenCreateChildNull() { + @Test + public void createChildShouldThrowNPEWhenChildIsNull() { //given - node = new NodeItem<>("subject"); + node = new NodeItem<>("subject", Node::getData); + exception.expect(NullPointerException.class); + exception.expectMessage("child"); //when node.createChild(null); } + @Test + public void getNameShouldBeCorrect() { + //given + node = new NodeItem<>("subject", Node::getData); + //then + assertThat(node.getName()).isEqualTo("subject"); + } + + @Test + public void getNameShouldUseParentNameSupplier() { + //given + val root = new NodeItem("root", Node::getData); + node = new NodeItem<>("child", root); + //then + assertThat(node.getName()).isEqualTo("child"); + } + + @Test + public void getNameShouldReturnNameForNonStringData() { + val root = new NodeItem(LocalDate.parse("2016-05-23"), + n -> n.getData().format(DateTimeFormatter.BASIC_ISO_DATE)); + //then + assertThat(root.getName()).isEqualTo("20160523"); + } + + @Test + public void getNameShouldUseClosestNameSupplier() { + node = new NodeItem<>("root", Node::getData); + val child = new NodeItem("child", Object::toString); + node.addChild(child); + val grandChild = new NodeItem<>("grandchild", child); + //then + assertThat(node.getName()).isEqualTo("root"); + assertThat(child.getName()).isNotEqualTo("child"); + assertThat(grandChild.getName()).isNotEqualTo("grandchild"); + } + + @Test + public void getNameShouldWorkWithoutNameSupplier() { + node = new NodeItem<>(null, "root"); + val namedchild = new NodeItem<>("named", "Alice", node); + //then + assertThat(node.getName()).isEqualTo("root"); + assertThat(namedchild.getName()).isEqualTo("Alice"); + } + + @Test + public void canCreateRootNodeWithoutData() { + node = new NodeItem<>(null, "empty"); + assertThat(node.getData()).isNull(); + } + + @Test + public void canCreateRootNodeWithoutDataButWithNameSupplier() { + node = new NodeItem<>(null, Node::getData); + assertThat(node.getData()).isNull(); + } + + @Test + public void getChildNamedFindsChild() { + //given + node = new NodeItem<>(null, "root"); + val alpha = new NodeItem(null, "alpha"); + val beta = new NodeItem(null, "beta"); + node.addChild(alpha); + node.addChild(beta); + //when + val result = node.getChildNamed("alpha"); + //then + assertThat(result).isSameAs(alpha); + } + + @Test + public void getChildNamedFindsNothing() { + //given + node = new NodeItem<>(null, "root"); + val alpha = new NodeItem(null, "alpha"); + val beta = new NodeItem(null, "beta"); + node.addChild(alpha); + node.addChild(beta); + exception.expect(NodeException.class); + exception.expectMessage("Named child not found"); + //when + node.getChildNamed("gamma"); + } + + @Test + public void nodeNamesAreUniqueWithinAParent() { + //given + node = new NodeItem<>(null, "root"); + val alpha = new NodeItem(null, "alpha"); + node.addChild(alpha); + val beta = new NodeItem(null, "alpha"); + exception.expect(NodeException.class); + exception.expectMessage("Node with that name already exists here"); + //when + node.addChild(beta); + } + + @Test + public void canPlaceNodeInTreeByPathNames() { + //given + node = new NodeItem<>(null, "root"); // create a root + val four = new NodeItem("data", "four"); + //when + node.placeNodeIn(four, "one", "two", "three"); + //then + val three = four.getParent(); + assertThat(four.getParent()).as("add node to a tree").isNotNull(); + assertThat(three.getName()).isEqualTo("three"); + val two = three.getParent(); + assertThat(two.getName()).isEqualTo("two"); + val one = two.getParent(); + assertThat(one.getName()).isEqualTo("one"); + assertThat(one.getParent()).isSameAs(node); + assertThat(node.getChildNamed("one") + .getChildNamed("two") + .getChildNamed("three") + .getChildNamed("four")).isSameAs(four); + } + + @Test + @SuppressWarnings("unchecked") + public void canPlaceInTreeUnderExistingNode() { + //given + node = new NodeItem<>(null, "root"); + val child = new NodeItem("child data", "child"); + val grandchild = new NodeItem("grandchild data", "grandchild"); + //when + node.placeNodeIn(child); // as root/child + node.placeNodeIn(grandchild, "child"); // as root/child/grandchild + //then + assertThat(node.getChildNamed("child")).as("child").isSameAs(child); + assertThat(node.getChildNamed("child").getChildNamed("grandchild")).as( + "grandchild").isSameAs(grandchild); + } + + @Test + @SuppressWarnings("unchecked") + public void canPlaceInTreeAboveExistingNode() { + //given + node = new NodeItem<>(null, "root"); + val child = new NodeItem("child data", "child"); + val grandchild = new NodeItem("grandchild data", "grandchild"); + //when + node.placeNodeIn(grandchild, "child"); + node.placeNodeIn(child); + //then + assertThat(node.getChildNamed("child")).as("child").isSameAs(child); + assertThat(node.getChildNamed("child").getChildNamed("grandchild")).as( + "grandchild").isSameAs(grandchild); + } + + @Test + public void removingParentFromNodeWithNoParentIsNoop() { + //given + node = new NodeItem<>(null); + //when + node.removeParent(); + } } From aaab7bbe67beefe24736c909fe71768fbac7c917 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 11:55:49 +0100 Subject: [PATCH 10/13] Reorganise code --- src/main/java/net/kemitix/node/Node.java | 14 +- src/main/java/net/kemitix/node/NodeItem.java | 192 +++++++++---------- 2 files changed, 103 insertions(+), 103 deletions(-) diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index 5ce9d56..a8dccdf 100644 --- a/src/main/java/net/kemitix/node/Node.java +++ b/src/main/java/net/kemitix/node/Node.java @@ -51,6 +51,13 @@ public interface Node { */ Node getParent(); + /** + * Make the current node a direct child of the parent. + * + * @param parent the new parent node + */ + void setParent(final Node parent); + /** * Fetches the child nodes. * @@ -110,13 +117,6 @@ public interface Node { */ boolean isChildOf(final Node node); - /** - * Make the current node a direct child of the parent. - * - * @param parent the new parent node - */ - void setParent(final Node parent); - /** * Walks the node tree using the path to select each child. * diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index bc87179..a756483 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -26,16 +26,6 @@ public class NodeItem implements Node { private String name; - /** - * Create unnamed root node. - * - * @param data the data or null - */ - public NodeItem(final T data) { - this.data = data; - this.nameSupplier = (n) -> null; - } - /** * Create named root node. * @@ -47,6 +37,16 @@ public class NodeItem implements Node { this.name = name; } + /** + * Create unnamed root node. + * + * @param data the data or null + */ + public NodeItem(final T data) { + this.data = data; + this.nameSupplier = (n) -> null; + } + /** * Creates root node with a name supplier. * @@ -142,26 +142,6 @@ public class NodeItem implements Node { return children; } - /** - * Make the current node a direct child of the parent. - * - * @param parent the new parent node - */ - @Override - public final void setParent(final Node parent) { - if (parent == null) { - throw new NullPointerException("parent"); - } - if (this.equals(parent) || parent.isChildOf(this)) { - throw new NodeException("Parent is a descendant"); - } - if (this.parent != null) { - this.parent.getChildren().remove(this); - } - this.parent = parent; - parent.addChild(this); - } - /** * Adds the child to the node. * @@ -190,40 +170,18 @@ public class NodeItem implements Node { } /** - * Checks if the node is an ancestor. + * Creates a new node and adds it as a child of the current node. * - * @param node the potential ancestor + * @param child the child node's data * - * @return true if the node is an ancestor + * @return the new child node */ @Override - public boolean isChildOf(final Node node) { - return parent != null && (node.equals(parent) || parent.isChildOf( - 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> walkTree(final List path) { - if (path == null) { - throw new NullPointerException("path"); + public Node createChild(final T child) { + if (child == null) { + throw new NullPointerException("child"); } - if (path.size() > 0) { - Optional> found = getChild(path.get(0)); - if (found.isPresent()) { - if (path.size() > 1) { - return found.get().walkTree(path.subList(1, path.size())); - } - return found; - } - } - return Optional.empty(); + return new NodeItem<>(child, this); } /** @@ -277,57 +235,60 @@ public class NodeItem implements Node { } /** - * Creates a new node and adds it as a child of the current node. + * Checks if the node is an ancestor. * - * @param child the child node's data + * @param node the potential ancestor * - * @return the new child node + * @return true if the node is an ancestor */ @Override - public Node createChild(final T child) { - if (child == null) { - throw new NullPointerException("child"); - } - return new NodeItem<>(child, this); + public boolean isChildOf(final Node node) { + return parent != null && (node.equals(parent) || parent.isChildOf( + node)); } + /** + * Make the current node a direct child of the parent. + * + * @param parent the new parent node + */ @Override - public Optional> findChildNamed(final String named) { - if (named == null) { - throw new NullPointerException("name"); + public final void setParent(final Node parent) { + if (parent == null) { + throw new NullPointerException("parent"); } - return children.stream() - .filter((Node t) -> t.getName().equals(named)) - .findAny(); + if (this.equals(parent) || parent.isChildOf(this)) { + throw new NodeException("Parent is a descendant"); + } + if (this.parent != null) { + this.parent.getChildren().remove(this); + } + this.parent = parent; + parent.addChild(this); } + /** + * 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 Node getChildNamed(final String named) { - final Optional> optional = findChildNamed(named); - if (optional.isPresent()) { - return optional.get(); + public Optional> walkTree(final List path) { + if (path == null) { + throw new NullPointerException("path"); } - throw new NodeException("Named child not found"); - } - - @Override - public void removeChild(final Node node) { - if (children.remove(node)) { - node.removeParent(); - } - } - - @Override - public void removeParent() { - if (parent != null) { - parent.removeChild(this); - parent = null; - if (nameSupplier == null) { - // this is now a root node, so must provide a default name - // supplier - nameSupplier = n -> null; + if (path.size() > 0) { + Optional> found = getChild(path.get(0)); + if (found.isPresent()) { + if (path.size() > 1) { + return found.get().walkTree(path.subList(1, path.size())); + } + return found; } } + return Optional.empty(); } @Override @@ -370,6 +331,25 @@ public class NodeItem implements Node { child.placeNodeIn(nodeItem, Arrays.copyOfRange(path, 1, path.length)); } + @Override + public Optional> findChildNamed(final String named) { + if (named == null) { + throw new NullPointerException("name"); + } + return children.stream() + .filter((Node t) -> t.getName().equals(named)) + .findAny(); + } + + @Override + public Node getChildNamed(final String named) { + final Optional> optional = findChildNamed(named); + if (optional.isPresent()) { + return optional.get(); + } + throw new NodeException("Named child not found"); + } + @Override public String drawTree(final int depth) { final StringBuilder sb = new StringBuilder(); @@ -390,4 +370,24 @@ public class NodeItem implements Node { return name != null && name.length() > 0; } + @Override + public void removeChild(final Node node) { + if (children.remove(node)) { + node.removeParent(); + } + } + + @Override + public void removeParent() { + if (parent != null) { + parent.removeChild(this); + parent = null; + if (nameSupplier == null) { + // this is now a root node, so must provide a default name + // supplier + nameSupplier = n -> null; + } + } + } + } From d82a7d6e9ffcbca13f82ce3bf68f61e97679a366 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 13:13:55 +0100 Subject: [PATCH 11/13] NoteItem: replace empty target node properly --- src/main/java/net/kemitix/node/NodeItem.java | 42 +++++++--------- .../java/net/kemitix/node/NodeItemTest.java | 49 +++++++++++++++++++ 2 files changed, 68 insertions(+), 23 deletions(-) diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index a756483..b77da2a 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -294,24 +294,26 @@ public class NodeItem implements Node { @Override public void placeNodeIn(final Node nodeItem, final String... path) { if (path.length == 0) { - if (nodeItem.isNamed()) { - final Optional> childNamed = findChildNamed( - nodeItem.getName()); - if (childNamed.isPresent()) { - final Node existing = childNamed.get(); - if (!existing.isEmpty()) { - throw new NodeException( - "A non-empty node with that name already " - + "exists here"); - } else { - existing.getChildren().forEach(nodeItem::addChild); - existing.removeParent(); - nodeItem.setParent(this); - return; - } - } + if (!nodeItem.isNamed()) { // nothing to conflict with + addChild(nodeItem); + return; + } + final Optional> childNamed = findChildNamed( + nodeItem.getName()); + 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 with that name already exists here"); + } else { + existing.getChildren().forEach(nodeItem::addChild); + existing.removeParent(); + addChild(nodeItem); } - addChild(nodeItem); return; } String item = path[0]; @@ -321,12 +323,6 @@ public class NodeItem implements Node { child = new NodeItem<>(null, item, this); } else { child = childNamed.get(); - if (child.isEmpty()) { - if (path.length == 1) { - getChildren().forEach(nodeItem::addChild); - nodeItem.setParent(this); - } - } } child.placeNodeIn(nodeItem, Arrays.copyOfRange(path, 1, path.length)); } diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java index 623ea94..5f5a5b0 100644 --- a/src/test/java/net/kemitix/node/NodeItemTest.java +++ b/src/test/java/net/kemitix/node/NodeItemTest.java @@ -744,4 +744,53 @@ public class NodeItemTest { //when node.removeParent(); } + + @Test + public void placeNodeInTreeWhereNonEmptyNodeWithSameNameExists() { + //given + exception.expect(NodeException.class); + exception.expectMessage( + "A non-empty node with that name already exists here"); + node = new NodeItem<>(null); + val child = new NodeItem(null, "child", node); + new NodeItem<>("data", "grandchild", child); + // root -> child -> grandchild + // only grandchild has data + //when + // attempt to add another node called 'grandchild' to 'child' + node.placeNodeIn(new NodeItem<>("cuckoo", "grandchild"), "child"); + } + + @Test + @SuppressWarnings("unchecked") + public void placeNodeInTreeWhenAddedNodeIsUnnamed() { + //given + node = new NodeItem<>(null); + final Node newNode = new NodeItem<>(null); + //when + node.placeNodeIn(newNode); + //then + assertThat(node.getChildren()).containsOnly(newNode); + } + + @Test + @SuppressWarnings("unchecked") + public void placeNodeInTreeWhenEmptyChildWithTargetNameExists() { + //given + node = new NodeItem<>(null); + final NodeItem child = new NodeItem<>(null, "child"); + final NodeItem target = new NodeItem<>(null, "target"); + node.addChild(child); + child.addChild(target); + final NodeItem addMe = new NodeItem<>("I'm new", "target"); + assertThat(addMe.getParent()).isNull(); + //when + // addMe should replace target as the sole descendant of child + node.placeNodeIn(addMe, "child"); + //then + assertThat(child.getChildren()).as("child only contains new node") + .containsOnly(addMe); + assertThat(target.getParent()).as("old node is removed from tree") + .isNull(); + } } From 6f81a62162d27c43ca74f05cf2e522eba794f636 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 13:33:53 +0100 Subject: [PATCH 12/13] NodeItem.removeParent(): use same name supplier in the new root node --- src/main/java/net/kemitix/node/NodeItem.java | 8 ++- .../java/net/kemitix/node/NodeItemTest.java | 63 +++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index b77da2a..8ef24a6 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -376,12 +376,14 @@ public class NodeItem implements Node { @Override public void removeParent() { if (parent != null) { - parent.removeChild(this); + Node oldParent = parent; + Function, String> supplier = getNameSupplier(); parent = null; - if (nameSupplier == null) { + oldParent.removeChild(this); + if (this.nameSupplier == null) { // this is now a root node, so must provide a default name // supplier - nameSupplier = n -> null; + this.nameSupplier = supplier; } } } diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java index 5f5a5b0..ec9953f 100644 --- a/src/test/java/net/kemitix/node/NodeItemTest.java +++ b/src/test/java/net/kemitix/node/NodeItemTest.java @@ -793,4 +793,67 @@ public class NodeItemTest { assertThat(target.getParent()).as("old node is removed from tree") .isNull(); } + + @Test + public void findChildNamedShouldThrowNPEWhenNameIsNull() { + //given + exception.expect(NullPointerException.class); + exception.expectMessage("name"); + node = new NodeItem<>(null); + //when + node.findChildNamed(null); + } + + @Test + public void isNamedNull() { + //given + node = new NodeItem<>(null); + //then + assertThat(node.isNamed()).isFalse(); + } + + @Test + public void isNamedEmpty() { + //given + node = new NodeItem<>(null, ""); + //then + assertThat(node.isNamed()).isFalse(); + } + + @Test + public void isNamedNamed() { + //given + node = new NodeItem<>(null, "named"); + //then + assertThat(node.isNamed()).isTrue(); + } + + @Test + public void removeParentNodeProvidesSameNameSupplier() { + // once a node has it's parent removed it should provide a default name + // provider + //given + node = new NodeItem<>("data", Node::getData); // name provider: getData + final NodeItem child = new NodeItem<>("other", node); + assertThat(node.getName()).as("initial root name").isEqualTo("data"); + assertThat(child.getName()).as("initial child name").isEqualTo("other"); + //when + child.removeParent(); + //then + assertThat(node.getName()).as("final root name").isEqualTo("data"); + assertThat(child.getName()).as("final child name").isEqualTo("other"); + } + + @Test + @SuppressWarnings("unchecked") + public void removeChildRemovesTheChild() { + //given + node = new NodeItem<>(null); + Node child = node.createChild("child"); + assertThat(node.getChildren()).containsExactly(child); + //then + node.removeChild(child); + //then + assertThat(node.getChildren()).isEmpty(); + } } From 3053eab9b038f889a4071e66616e3cce869fc5f6 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Tue, 24 May 2016 13:50:38 +0100 Subject: [PATCH 13/13] NodeItem.drawTree(): fix typo --- src/main/java/net/kemitix/node/NodeItem.java | 5 +++-- .../java/net/kemitix/node/NodeItemTest.java | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index 8ef24a6..1ef3bf6 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -354,8 +354,9 @@ public class NodeItem implements Node { sb.append(String.format("[%1$" + (depth + name.length()) + "s]\n", name)); } else if (!children.isEmpty()) { - sb.append(String.format("[%1$" + (depth + unnamed) + "s]\n", - unnamed)); + sb.append( + String.format("[%1$" + (depth + unnamed.length()) + "s]\n", + unnamed)); } getChildren().stream().forEach(c -> sb.append(c.drawTree(depth + 1))); return sb.toString(); diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java index ec9953f..200321f 100644 --- a/src/test/java/net/kemitix/node/NodeItemTest.java +++ b/src/test/java/net/kemitix/node/NodeItemTest.java @@ -856,4 +856,26 @@ public class NodeItemTest { //then assertThat(node.getChildren()).isEmpty(); } + + @Test + public void drawTreeIsCorrect() { + //given + node = new NodeItem<>(null, "root"); + val bob = new NodeItem(null, "bob", node); + val alice = new NodeItem(null, "alice", node); + new NodeItem<>(null, "dave", alice); + new NodeItem<>(null, bob); // has no name and no children so no included + val kim = new NodeItem(null, node); // nameless mother + new NodeItem<>(null, "lucy", kim); + //when + val tree = node.drawTree(0); + //then + String[] lines = tree.split("\n"); + assertThat(lines).contains("[root]", "[ alice]", "[ dave]", + "[ (unnamed)]", "[ lucy]", "[ bob]"); + assertThat(lines).containsSubsequence("[root]", "[ alice]", "[ dave]"); + assertThat(lines).containsSubsequence("[root]", "[ (unnamed)]", + "[ lucy]"); + assertThat(lines).containsSubsequence("[root]", "[ bob]"); + } }