From 41bd84b6f6654b0a71402b56e59c961267ba71e1 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Thu, 26 May 2016 13:03:25 +0100 Subject: [PATCH 1/6] Node.getData(): nodes may be empty so return an Optional Most tests used Node::getData as a name supplier for test fixtures. In some cases these were not needed and have been removed. In others, where there are used, they have been updated to get the content of the Optional. If the Optional happens to be empty, which it shouldn't be in those cases, then an error will occur and the test will, correctly, fail. --- src/main/java/net/kemitix/node/Node.java | 5 +- src/main/java/net/kemitix/node/NodeItem.java | 9 +- .../java/net/kemitix/node/NodeItemTest.java | 120 ++++++++++-------- 3 files changed, 75 insertions(+), 59 deletions(-) diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index 9d82efd..88b6400 100644 --- a/src/main/java/net/kemitix/node/Node.java +++ b/src/main/java/net/kemitix/node/Node.java @@ -32,9 +32,10 @@ public interface Node { /** * Fetch the data held within the node. * - * @return the node's data + * @return an Optional containing the node's data, or empty if the node has + * none */ - T getData(); + Optional getData(); /** * Set the data held within the node. diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index 357d040..956678c 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -124,8 +124,8 @@ public class NodeItem implements Node { } @Override - public T getData() { - return data; + public Optional getData() { + return Optional.ofNullable(data); } @Override @@ -244,7 +244,8 @@ public class NodeItem implements Node { throw new NullPointerException("child"); } return children.stream() - .filter((Node t) -> t.getData().equals(child)) + .filter(n -> !n.isEmpty()) + .filter(n -> n.getData().get().equals(child)) .findAny(); } @@ -333,7 +334,7 @@ public class NodeItem implements Node { throw new NodeException("A non-empty node named '" + nodeName + "' already exists here"); } else { - existing.setData(nodeItem.getData()); + nodeItem.getData().ifPresent(existing::setData); } return; } diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java index bd3c4ce..c1a690a 100644 --- a/src/test/java/net/kemitix/node/NodeItemTest.java +++ b/src/test/java/net/kemitix/node/NodeItemTest.java @@ -36,7 +36,7 @@ public class NodeItemTest { node = new NodeItem<>(data); //then assertThat(node.getData()).as("can get the data from a node"). - isSameAs(data); + contains(data); } @Test @@ -76,7 +76,7 @@ public class NodeItemTest { @Test public void shouldHaveNullForDefaultParent() { //given - node = new NodeItem<>("data", Node::getData); + node = new NodeItem<>("data"); //then assertThat(node.getParent()).as( "node created without a parent has null as parent").isNull(); @@ -88,7 +88,7 @@ public class NodeItemTest { @Test public void shouldReturnNodeParent() { //given - val parent = new NodeItem("parent", Node::getData); + val parent = new NodeItem("parent"); //when node = new NodeItem<>("subject", parent); //then @@ -104,7 +104,7 @@ public class NodeItemTest { @Test public void setParentShouldThrowNodeExceptionWhenParentIsAChild() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); val child = new NodeItem("child", node); exception.expect(NodeException.class); exception.expectMessage("Parent is a descendant"); @@ -120,7 +120,7 @@ public class NodeItemTest { @SuppressWarnings("unchecked") public void shouldAddNewNodeAsChildToParent() { //given - val parent = new NodeItem("parent", Node::getData); + val parent = new NodeItem("parent"); //when node = new NodeItem<>("subject", parent); //then @@ -135,8 +135,8 @@ public class NodeItemTest { @Test public void shouldReturnSetParent() { //given - node = new NodeItem<>("subject", Node::getData); - val parent = new NodeItem("parent", Node::getData); + node = new NodeItem<>("subject"); + val parent = new NodeItem("parent"); //when node.setParent(parent); //then @@ -151,7 +151,7 @@ public class NodeItemTest { @Test public void shouldThrowNPEWhenSetParentNull() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); exception.expect(NullPointerException.class); exception.expectMessage("parent"); //when @@ -165,7 +165,7 @@ public class NodeItemTest { @Test public void setParentShouldThrowNodeExceptionWhenParentIsSelf() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); exception.expect(NodeException.class); exception.expectMessage("Parent is a descendant"); //when @@ -179,9 +179,9 @@ public class NodeItemTest { @Test public void shouldUpdateOldParentWhenNodeSetToNewParent() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); val child = node.createChild("child"); - val newParent = new NodeItem("newParent", Node::getData); + val newParent = new NodeItem("newParent"); //when child.setParent(newParent); //then @@ -200,9 +200,9 @@ public class NodeItemTest { @Test public void shouldRemoveNodeFromOldParentWhenAddedAsChildToNewParent() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); val child = node.createChild("child"); - val newParent = new NodeItem("newParent", Node::getData); + val newParent = new NodeItem("newParent"); //when newParent.addChild(child); //then @@ -222,7 +222,7 @@ public class NodeItemTest { @Test public void shouldThrowNPEWhenAddingNullAsChild() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); exception.expect(NullPointerException.class); exception.expectMessage("child"); //when @@ -236,8 +236,8 @@ public class NodeItemTest { @SuppressWarnings("unchecked") public void shouldReturnAddedChild() { //given - node = new NodeItem<>("subject", Node::getData); - val child = new NodeItem("child", Node::getData); + node = new NodeItem<>("subject"); + val child = new NodeItem("child"); //when node.addChild(child); //then @@ -252,7 +252,7 @@ public class NodeItemTest { @Test public void addChildShouldThrowNodeExceptionWhenAddingANodeAsOwnChild() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); exception.expect(NodeException.class); exception.expectMessage("Child is an ancestor"); //then @@ -265,7 +265,7 @@ public class NodeItemTest { @Test public void addChildShouldThrowNodeExceptionWhenAddingSelfAsChild() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); exception.expect(NodeException.class); exception.expectMessage("Child is an ancestor"); //when @@ -279,7 +279,7 @@ public class NodeItemTest { @Test public void addChildShouldThrowNodeExceptionWhenChildIsParent() { //given - val parent = new NodeItem("parent", Node::getData); + val parent = new NodeItem("parent"); node = new NodeItem<>("subject", parent); exception.expect(NodeException.class); exception.expectMessage("Child is an ancestor"); @@ -294,7 +294,7 @@ public class NodeItemTest { @Test public void addChildShouldThrowNodeExceptionWhenAddingGrandParentAsChild() { //given - val grandParent = new NodeItem("grandparent", Node::getData); + val grandParent = new NodeItem("grandparent"); val parent = new NodeItem("parent", grandParent); node = new NodeItem<>("subject", parent); exception.expect(NodeException.class); @@ -309,8 +309,8 @@ public class NodeItemTest { @Test public void shouldSetParentOnChildWhenAddedAsChild() { //given - val child = new NodeItem("child", Node::getData); - node = new NodeItem<>("subject", Node::getData); + val child = new NodeItem("child"); + node = new NodeItem<>("subject"); //when node.addChild(child); //then @@ -326,7 +326,7 @@ public class NodeItemTest { public void shouldWalkTreeToNode() { //given val grandparent = "grandparent"; - val grandParentNode = new NodeItem(grandparent, Node::getData); + val grandParentNode = new NodeItem(grandparent); val parent = "parent"; val parentNode = new NodeItem(parent, grandParentNode); val subject = "subject"; @@ -351,7 +351,7 @@ public class NodeItemTest { public void shouldNotFindNonExistentChildNode() { //given val parent = "parent"; - val parentNode = new NodeItem(parent, Node::getData); + val parentNode = new NodeItem(parent); val subject = "subject"; node = new NodeItem<>(subject, parentNode); //when @@ -368,7 +368,7 @@ public class NodeItemTest { @Test public void shouldThrowNEWhenWalkTreeNull() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); exception.expect(NullPointerException.class); exception.expectMessage("path"); //when @@ -382,7 +382,7 @@ public class NodeItemTest { @Test public void shouldReturnEmptyForEmptyWalkTreePath() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); //when val result = node.findInPath(Collections.emptyList()); //then @@ -395,7 +395,7 @@ public class NodeItemTest { @Test public void shouldCreateDescendantNodes() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); val alphaData = "alpha"; val betaData = "beta"; val gammaData = "gamma"; @@ -444,7 +444,7 @@ public class NodeItemTest { @Test public void createDescendantLineShouldThrowNPEWhenDescendantsAreNull() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); exception.expect(NullPointerException.class); exception.expectMessage("descendants"); //when @@ -457,7 +457,7 @@ public class NodeItemTest { @Test public void shouldChangeNothingWhenCreateDescendantEmpty() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); //when node.createDescendantLine(Collections.emptyList()); //then @@ -472,7 +472,7 @@ public class NodeItemTest { @Test public void shouldFindExistingChildNode() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); val childData = "child"; val child = new NodeItem(childData, node); //when @@ -489,14 +489,14 @@ public class NodeItemTest { @Test public void shouldFindCreateNewChildNode() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); val childData = "child"; //when val found = node.findOrCreateChild(childData); //then assertThat(found.getData()).as( - "when searching for a child by data, a new node is created") - .isSameAs(childData); + "when searching for a non-existent child by data, a new node " + + "is created").contains(childData); } /** @@ -505,7 +505,7 @@ public class NodeItemTest { @Test public void findOrCreateChildShouldThrowNPEFWhenChildIsNull() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); exception.expect(NullPointerException.class); exception.expectMessage("child"); //when @@ -518,9 +518,9 @@ public class NodeItemTest { @Test public void shouldGetChild() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); val childData = "child"; - val child = new NodeItem(childData, Node::getData); + val child = new NodeItem(childData); node.addChild(child); //when val found = node.findChild(childData); @@ -540,7 +540,7 @@ public class NodeItemTest { @Test public void getChildShouldThrowNPEWhenThereIsNoChild() { //given - node = new NodeItem<>("data", Node::getData); + node = new NodeItem<>("data"); exception.expect(NullPointerException.class); exception.expectMessage("child"); //when @@ -554,7 +554,7 @@ public class NodeItemTest { @Test public void shouldCreateChild() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); val childData = "child"; //when val child = node.createChild(childData); @@ -579,7 +579,7 @@ public class NodeItemTest { @Test public void createChildShouldThrowNPEWhenChildIsNull() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject"); exception.expect(NullPointerException.class); exception.expectMessage("child"); //when @@ -589,7 +589,7 @@ public class NodeItemTest { @Test public void getNameShouldBeCorrect() { //given - node = new NodeItem<>("subject", Node::getData); + node = new NodeItem<>("subject", n -> n.getData().get()); //then assertThat(node.getName()).isEqualTo("subject"); } @@ -597,7 +597,7 @@ public class NodeItemTest { @Test public void getNameShouldUseParentNameSupplier() { //given - val root = new NodeItem("root", Node::getData); + val root = new NodeItem("root", n -> n.getData().get()); node = new NodeItem<>("child", root); //then assertThat(node.getName()).isEqualTo("child"); @@ -605,15 +605,20 @@ public class NodeItemTest { @Test public void getNameShouldReturnNameForNonStringData() { - val root = new NodeItem(LocalDate.parse("2016-05-23"), - n -> n.getData().format(DateTimeFormatter.BASIC_ISO_DATE)); + val root = new NodeItem(LocalDate.parse("2016-05-23"), n -> { + if (n.isEmpty()) { + return null; + } + return n.getData().get().format(DateTimeFormatter.BASIC_ISO_DATE); + + }); //then assertThat(root.getName()).isEqualTo("20160523"); } @Test public void getNameShouldUseClosestNameSupplier() { - node = new NodeItem<>("root", Node::getData); + node = new NodeItem<>("root", n -> n.getData().get()); val child = new NodeItem("child", Object::toString); node.addChild(child); val grandChild = new NodeItem<>("grandchild", child); @@ -635,13 +640,13 @@ public class NodeItemTest { @Test public void canCreateRootNodeWithoutData() { node = new NodeItem<>(null, "empty"); - assertThat(node.getData()).isNull(); + assertThat(node.getData()).isEmpty(); } @Test public void canCreateRootNodeWithoutDataButWithNameSupplier() { - node = new NodeItem<>(null, Node::getData); - assertThat(node.getData()).isNull(); + node = new NodeItem<>(null); + assertThat(node.getData()).isEmpty(); } @Test @@ -736,7 +741,7 @@ public class NodeItemTest { node.insertInPath(child); //then assertThat(node.getChildByName("child").getData()).as("data in tree") - .isSameAs( + .contains( "child data"); assertThat( node.getChildByName("child").getChildByName("grandchild")).as( @@ -808,7 +813,7 @@ public class NodeItemTest { node.insertInPath(addMe, "child"); //then assertThat(child.getChildByName("target").getData()).as( - "target now contains data").isEqualTo("I'm new"); + "target now contains data").contains("I'm new"); } @Test @@ -850,7 +855,7 @@ public class NodeItemTest { // 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 + node = new NodeItem<>("data", n -> n.getData().get()); 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"); @@ -903,7 +908,7 @@ public class NodeItemTest { //when node.setData("updated"); //then - assertThat(node.getData()).isEqualTo("updated"); + assertThat(node.getData()).contains("updated"); } @Test @@ -946,7 +951,7 @@ public class NodeItemTest { //given node = new NodeItem<>(null); //when - NodeItem child = new NodeItem<>(null, Node::getData, node); + NodeItem child = new NodeItem<>(null, node); //then assertThat(child.getParent()).isSameAs(node); assertThat(node.getChildren()).containsExactly(child); @@ -1023,7 +1028,7 @@ public class NodeItemTest { if (parent == null) { return ""; } - return parent.getName() + "/" + node.getData(); + return parent.getName() + "/" + node.getData().get(); }; node = new NodeItem<>(null, pathNameSupplier); val child = new NodeItem("child", node); @@ -1031,4 +1036,13 @@ public class NodeItemTest { //then assertThat(grandchild.getName()).isEqualTo("/child/grandchild"); } + + @Test + public void canSafelyHandleFindChildWhenAChildHasNoData() { + //given + node = new NodeItem<>(null); + new NodeItem<>(null, node); + //when + node.findChild("data"); + } } From 37562941f4d7225b98c0750d79b3700b902235d3 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Thu, 26 May 2016 12:22:20 +0100 Subject: [PATCH 2/6] Node.getChildByName(): javadoc to note throws NodeException if child not found --- src/main/java/net/kemitix/node/Node.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index 88b6400..f6e2de1 100644 --- a/src/main/java/net/kemitix/node/Node.java +++ b/src/main/java/net/kemitix/node/Node.java @@ -183,6 +183,8 @@ public interface Node { * @param name the name of the child * * @return the node + * + * @throws NodeException if the node is not found */ Node getChildByName(String name); From 6c13cd2735c04d9f58f95380a14707810348290c Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Thu, 26 May 2016 12:57:56 +0100 Subject: [PATCH 3/6] Node.getParent(): returns any parent inside an Optional Root nodes will return an empty Optional. --- src/main/java/net/kemitix/node/Node.java | 7 +-- src/main/java/net/kemitix/node/NodeItem.java | 11 +++-- .../java/net/kemitix/node/NodeItemTest.java | 46 +++++++++---------- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index f6e2de1..7cda730 100644 --- a/src/main/java/net/kemitix/node/Node.java +++ b/src/main/java/net/kemitix/node/Node.java @@ -53,13 +53,10 @@ public interface Node { /** * Fetch the parent node. - *

- * If the node is a root node, i.e. has no parent, then this will return - * null. * - * @return the parent node + * @return an Optional contain the parent node, or empty if a root node */ - Node getParent(); + Optional> getParent(); /** * Make the current node a direct child of the parent. diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index 956678c..ec71f11 100644 --- a/src/main/java/net/kemitix/node/NodeItem.java +++ b/src/main/java/net/kemitix/node/NodeItem.java @@ -139,8 +139,8 @@ public class NodeItem implements Node { } @Override - public Node getParent() { - return parent; + public Optional> getParent() { + return Optional.ofNullable(parent); } @Override @@ -170,7 +170,12 @@ public class NodeItem implements Node { } } children.add(child); - if (child.getParent() == null || !child.getParent().equals(this)) { + // 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) { child.setParent(this); } } diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java index c1a690a..68770a2 100644 --- a/src/test/java/net/kemitix/node/NodeItemTest.java +++ b/src/test/java/net/kemitix/node/NodeItemTest.java @@ -79,7 +79,7 @@ public class NodeItemTest { node = new NodeItem<>("data"); //then assertThat(node.getParent()).as( - "node created without a parent has null as parent").isNull(); + "node created without a parent has no parent").isEmpty(); } /** @@ -94,7 +94,7 @@ public class NodeItemTest { //then assertThat(node.getParent()).as( "node created with a parent can return the parent") - .isSameAs(parent); + .contains(parent); } /** @@ -142,7 +142,7 @@ public class NodeItemTest { //then assertThat(node.getParent()).as( "when a node is assigned a new parent that parent can be " - + "returned").isSameAs(parent); + + "returned").contains(parent); } /** @@ -187,7 +187,7 @@ public class NodeItemTest { //then assertThat(child.getParent()).as( "when a node is assigned a new parent, the old parent is " - + "replaced").isSameAs(newParent); + + "replaced").contains(newParent); assertThat(node.findChild("child").isPresent()).as( "when a node is assigned a new parent, the old parent no " + "longer has the node among it's children").isFalse(); @@ -209,7 +209,7 @@ public class NodeItemTest { 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") - .isSameAs(newParent); + .contains(newParent); assertThat(node.findChild("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 " @@ -309,14 +309,14 @@ public class NodeItemTest { @Test public void shouldSetParentOnChildWhenAddedAsChild() { //given - val child = new NodeItem("child"); node = new NodeItem<>("subject"); + val child = new NodeItem("child"); //when node.addChild(child); //then assertThat(child.getParent()).as( "when a node is added as a child, the child has the node as " - + "its parent").isSameAs(node); + + "its parent").contains(node); } /** @@ -411,7 +411,7 @@ public class NodeItemTest { val alpha = alphaOptional.get(); assertThat(alpha.getParent()).as( "when creating a descendant line, the first element has " - + "the current node as its parent").isSameAs(node); + + "the current node as its parent").contains(node); val betaOptional = alpha.findChild(betaData); assertThat(betaOptional.isPresent()).as( "when creating a descendant line, the second element is " @@ -421,7 +421,7 @@ public class NodeItemTest { assertThat(beta.getParent()).as( "when creating a descendant line, the second element " + "has the first as its parent") - .isSameAs(alpha); + .contains(alpha); val gammaOptional = beta.findChild(gammaData); assertThat(gammaOptional.isPresent()).as( "when creating a descendant line, the third element " @@ -431,7 +431,7 @@ public class NodeItemTest { assertThat(gamma.getParent()).as( "when creating a descendant line, the third " + "element has the second as its parent") - .isSameAs(beta); + .contains(beta); } } } @@ -561,7 +561,7 @@ public class NodeItemTest { //then assertThat(child.getParent()).as( "when creating a child node, the child has the current node " - + "as its parent").isSameAs(node); + + "as its parent").contains(node); val foundChild = node.findChild(childData); assertThat(foundChild.isPresent()).as( "when creating a child node, the child can be found by its " @@ -698,14 +698,14 @@ public class NodeItemTest { //when node.insertInPath(four, "one", "two", "three"); //then - val three = four.getParent(); + val three = four.getParent().get(); assertThat(four.getParent()).as("add node to a tree").isNotNull(); assertThat(three.getName()).isEqualTo("three"); - val two = three.getParent(); + val two = three.getParent().get(); assertThat(two.getName()).isEqualTo("two"); - val one = two.getParent(); + val one = two.getParent().get(); assertThat(one.getName()).isEqualTo("one"); - assertThat(one.getParent()).isSameAs(node); + assertThat(one.getParent().get()).isSameAs(node); assertThat(node.getChildByName("one") .getChildByName("two") .getChildByName("three") @@ -764,7 +764,7 @@ public class NodeItemTest { //when child.removeParent(); //then - assertThat(child.getParent()).isNull(); + assertThat(child.getParent()).isEmpty(); } @Test @@ -805,7 +805,7 @@ public class NodeItemTest { node.addChild(child); child.addChild(target); final NodeItem addMe = new NodeItem<>("I'm new", "target"); - assertThat(addMe.getParent()).isNull(); + assertThat(addMe.getParent()).isEmpty(); assertThat(child.getChildByName("target").isEmpty()).as( "target starts empty").isTrue(); //when @@ -920,7 +920,7 @@ public class NodeItemTest { Node child = node.createChild("child data", "child name"); //then assertThat(child.getName()).isEqualTo("child name"); - assertThat(child.getParent()).isSameAs(node); + assertThat(child.getParent()).contains(node); assertThat(node.getChildren()).containsExactly(child); } @@ -953,7 +953,7 @@ public class NodeItemTest { //when NodeItem child = new NodeItem<>(null, node); //then - assertThat(child.getParent()).isSameAs(node); + assertThat(child.getParent()).contains(node); assertThat(node.getChildren()).containsExactly(child); } @@ -1024,11 +1024,11 @@ public class NodeItemTest { public void canUseNameSupplierToBuildFullPath() { //given final Function, String> pathNameSupplier = node -> { - Node parent = node.getParent(); - if (parent == null) { - return ""; + Optional> parent = node.getParent(); + if (parent.isPresent()) { + return parent.get().getName() + "/" + node.getData().get(); } - return parent.getName() + "/" + node.getData().get(); + return ""; }; node = new NodeItem<>(null, pathNameSupplier); val child = new NodeItem("child", node); From d3b8fa721b3151ad13943a370368d85fd0e56916 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Thu, 26 May 2016 13:08:52 +0100 Subject: [PATCH 4/6] NodeItemTest: check that parent is set when using NodeItem(data, supplier, parent) --- src/test/java/net/kemitix/node/NodeItemTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java index 68770a2..277b81a 100644 --- a/src/test/java/net/kemitix/node/NodeItemTest.java +++ b/src/test/java/net/kemitix/node/NodeItemTest.java @@ -55,9 +55,10 @@ public class NodeItemTest { //given node = new NodeItem<>(null, n -> "root name supplier"); //when - val child = new NodeItem<>(null, n -> "overridden", node); + val child = new NodeItem(null, n -> "overridden", node); //then assertThat(child.getName()).isEqualTo("overridden"); + assertThat(child.getParent()).contains(node); } @Test From 136bc0b4babdff6f5e9a8cc2b472bcefe883a7d1 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Thu, 26 May 2016 13:11:51 +0100 Subject: [PATCH 5/6] NodeItemTest: ensure that when a child is removed from it's parent its parent is removed --- src/test/java/net/kemitix/node/NodeItemTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java index 277b81a..283b042 100644 --- a/src/test/java/net/kemitix/node/NodeItemTest.java +++ b/src/test/java/net/kemitix/node/NodeItemTest.java @@ -878,6 +878,7 @@ public class NodeItemTest { node.removeChild(child); //then assertThat(node.getChildren()).isEmpty(); + assertThat(child.getParent()).isEmpty(); } @Test From 1ea7802768dd6041c60c4be267f5780284032737 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Thu, 26 May 2016 13:17:21 +0100 Subject: [PATCH 6/6] NodeItemTest: ensure that removeParent() removes the child from the parent too --- src/test/java/net/kemitix/node/NodeItemTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java index 283b042..700b053 100644 --- a/src/test/java/net/kemitix/node/NodeItemTest.java +++ b/src/test/java/net/kemitix/node/NodeItemTest.java @@ -761,11 +761,12 @@ public class NodeItemTest { public void removingParentFromNodeWithParentRemovesParent() { //given node = new NodeItem<>(null); - NodeItem child = new NodeItem<>(null, node); + val child = new NodeItem(null, node); //when child.removeParent(); //then assertThat(child.getParent()).isEmpty(); + assertThat(node.getChildren()).isEmpty(); } @Test