diff --git a/CHANGELOG b/CHANGELOG index 6b8d986..66ec612 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,11 @@ CHANGELOG ========= +0.3.0 +------ + +* Return optionals rather than nulls + 0.2.0 ------ diff --git a/pom.xml b/pom.xml index 248d924..1906316 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 node - 0.2.0 + 0.3.0 jar Node diff --git a/src/main/java/net/kemitix/node/Node.java b/src/main/java/net/kemitix/node/Node.java index 9d82efd..7cda730 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. @@ -52,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. @@ -182,6 +180,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); diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java index b724048..ec71f11 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 @@ -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); } } @@ -244,7 +249,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(); } @@ -321,8 +327,8 @@ public class NodeItem implements Node { addChild(nodeItem); return; } - final Optional> childNamed = findChildByName( - nodeItem.getName()); + String nodeName = nodeItem.getName(); + final Optional> childNamed = findChildByName(nodeName); if (!childNamed.isPresent()) { // nothing with the same name exists addChild(nodeItem); return; @@ -330,10 +336,10 @@ public class NodeItem implements Node { // 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"); + 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 b292c9d..700b053 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 @@ -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 @@ -76,10 +77,10 @@ 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(); + "node created without a parent has no parent").isEmpty(); } /** @@ -88,13 +89,13 @@ 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 assertThat(node.getParent()).as( "node created with a parent can return the parent") - .isSameAs(parent); + .contains(parent); } /** @@ -104,7 +105,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 +121,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,14 +136,14 @@ 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 assertThat(node.getParent()).as( "when a node is assigned a new parent that parent can be " - + "returned").isSameAs(parent); + + "returned").contains(parent); } /** @@ -151,7 +152,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 +166,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,15 +180,15 @@ 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 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(); @@ -200,16 +201,16 @@ 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 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 " @@ -222,7 +223,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 +237,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 +253,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 +266,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 +280,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 +295,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,14 +310,14 @@ public class NodeItemTest { @Test public void shouldSetParentOnChildWhenAddedAsChild() { //given - val child = new NodeItem("child", Node::getData); - node = new NodeItem<>("subject", Node::getData); + 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); } /** @@ -326,7 +327,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 +352,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 +369,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 +383,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 +396,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"; @@ -411,7 +412,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 +422,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 +432,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); } } } @@ -444,7 +445,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 +458,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 +473,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 +490,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 +506,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 +519,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 +541,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,14 +555,14 @@ 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); //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 " @@ -579,7 +580,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 +590,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 +598,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 +606,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 +641,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 @@ -693,14 +699,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") @@ -736,7 +742,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( @@ -755,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()).isNull(); + assertThat(child.getParent()).isEmpty(); + assertThat(node.getChildren()).isEmpty(); } @Test @@ -767,7 +774,7 @@ public class NodeItemTest { //given exception.expect(NodeException.class); exception.expectMessage( - "A non-empty node with that name already exists here"); + "A non-empty node named 'grandchild' already exists here"); node = new NodeItem<>(null); val child = new NodeItem(null, "child", node); new NodeItem<>("data", "grandchild", child); @@ -800,7 +807,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 @@ -808,7 +815,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 +857,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"); @@ -872,6 +879,7 @@ public class NodeItemTest { node.removeChild(child); //then assertThat(node.getChildren()).isEmpty(); + assertThat(child.getParent()).isEmpty(); } @Test @@ -903,7 +911,7 @@ public class NodeItemTest { //when node.setData("updated"); //then - assertThat(node.getData()).isEqualTo("updated"); + assertThat(node.getData()).contains("updated"); } @Test @@ -915,7 +923,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); } @@ -946,9 +954,9 @@ 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(child.getParent()).contains(node); assertThat(node.getChildren()).containsExactly(child); } @@ -1019,11 +1027,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(); + return ""; }; node = new NodeItem<>(null, pathNameSupplier); val child = new NodeItem("child", node); @@ -1031,4 +1039,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"); + } }