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.
This commit is contained in:
Paul Campbell 2016-05-26 13:03:25 +01:00
parent d0e6769126
commit 41bd84b6f6
3 changed files with 75 additions and 59 deletions

View file

@ -32,9 +32,10 @@ public interface Node<T> {
/** /**
* Fetch the data held within the 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<T> getData();
/** /**
* Set the data held within the node. * Set the data held within the node.

View file

@ -124,8 +124,8 @@ public class NodeItem<T> implements Node<T> {
} }
@Override @Override
public T getData() { public Optional<T> getData() {
return data; return Optional.ofNullable(data);
} }
@Override @Override
@ -244,7 +244,8 @@ public class NodeItem<T> implements Node<T> {
throw new NullPointerException("child"); throw new NullPointerException("child");
} }
return children.stream() return children.stream()
.filter((Node<T> t) -> t.getData().equals(child)) .filter(n -> !n.isEmpty())
.filter(n -> n.getData().get().equals(child))
.findAny(); .findAny();
} }
@ -333,7 +334,7 @@ public class NodeItem<T> implements Node<T> {
throw new NodeException("A non-empty node named '" + nodeName throw new NodeException("A non-empty node named '" + nodeName
+ "' already exists here"); + "' already exists here");
} else { } else {
existing.setData(nodeItem.getData()); nodeItem.getData().ifPresent(existing::setData);
} }
return; return;
} }

View file

@ -36,7 +36,7 @@ public class NodeItemTest {
node = new NodeItem<>(data); node = new NodeItem<>(data);
//then //then
assertThat(node.getData()).as("can get the data from a node"). assertThat(node.getData()).as("can get the data from a node").
isSameAs(data); contains(data);
} }
@Test @Test
@ -76,7 +76,7 @@ public class NodeItemTest {
@Test @Test
public void shouldHaveNullForDefaultParent() { public void shouldHaveNullForDefaultParent() {
//given //given
node = new NodeItem<>("data", Node::getData); node = new NodeItem<>("data");
//then //then
assertThat(node.getParent()).as( assertThat(node.getParent()).as(
"node created without a parent has null as parent").isNull(); "node created without a parent has null as parent").isNull();
@ -88,7 +88,7 @@ public class NodeItemTest {
@Test @Test
public void shouldReturnNodeParent() { public void shouldReturnNodeParent() {
//given //given
val parent = new NodeItem<String>("parent", Node::getData); val parent = new NodeItem<String>("parent");
//when //when
node = new NodeItem<>("subject", parent); node = new NodeItem<>("subject", parent);
//then //then
@ -104,7 +104,7 @@ public class NodeItemTest {
@Test @Test
public void setParentShouldThrowNodeExceptionWhenParentIsAChild() { public void setParentShouldThrowNodeExceptionWhenParentIsAChild() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
val child = new NodeItem<String>("child", node); val child = new NodeItem<String>("child", node);
exception.expect(NodeException.class); exception.expect(NodeException.class);
exception.expectMessage("Parent is a descendant"); exception.expectMessage("Parent is a descendant");
@ -120,7 +120,7 @@ public class NodeItemTest {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
public void shouldAddNewNodeAsChildToParent() { public void shouldAddNewNodeAsChildToParent() {
//given //given
val parent = new NodeItem<String>("parent", Node::getData); val parent = new NodeItem<String>("parent");
//when //when
node = new NodeItem<>("subject", parent); node = new NodeItem<>("subject", parent);
//then //then
@ -135,8 +135,8 @@ public class NodeItemTest {
@Test @Test
public void shouldReturnSetParent() { public void shouldReturnSetParent() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
val parent = new NodeItem<String>("parent", Node::getData); val parent = new NodeItem<String>("parent");
//when //when
node.setParent(parent); node.setParent(parent);
//then //then
@ -151,7 +151,7 @@ public class NodeItemTest {
@Test @Test
public void shouldThrowNPEWhenSetParentNull() { public void shouldThrowNPEWhenSetParentNull() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
exception.expect(NullPointerException.class); exception.expect(NullPointerException.class);
exception.expectMessage("parent"); exception.expectMessage("parent");
//when //when
@ -165,7 +165,7 @@ public class NodeItemTest {
@Test @Test
public void setParentShouldThrowNodeExceptionWhenParentIsSelf() { public void setParentShouldThrowNodeExceptionWhenParentIsSelf() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
exception.expect(NodeException.class); exception.expect(NodeException.class);
exception.expectMessage("Parent is a descendant"); exception.expectMessage("Parent is a descendant");
//when //when
@ -179,9 +179,9 @@ public class NodeItemTest {
@Test @Test
public void shouldUpdateOldParentWhenNodeSetToNewParent() { public void shouldUpdateOldParentWhenNodeSetToNewParent() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
val child = node.createChild("child"); val child = node.createChild("child");
val newParent = new NodeItem<String>("newParent", Node::getData); val newParent = new NodeItem<String>("newParent");
//when //when
child.setParent(newParent); child.setParent(newParent);
//then //then
@ -200,9 +200,9 @@ public class NodeItemTest {
@Test @Test
public void shouldRemoveNodeFromOldParentWhenAddedAsChildToNewParent() { public void shouldRemoveNodeFromOldParentWhenAddedAsChildToNewParent() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
val child = node.createChild("child"); val child = node.createChild("child");
val newParent = new NodeItem<String>("newParent", Node::getData); val newParent = new NodeItem<String>("newParent");
//when //when
newParent.addChild(child); newParent.addChild(child);
//then //then
@ -222,7 +222,7 @@ public class NodeItemTest {
@Test @Test
public void shouldThrowNPEWhenAddingNullAsChild() { public void shouldThrowNPEWhenAddingNullAsChild() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
exception.expect(NullPointerException.class); exception.expect(NullPointerException.class);
exception.expectMessage("child"); exception.expectMessage("child");
//when //when
@ -236,8 +236,8 @@ public class NodeItemTest {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
public void shouldReturnAddedChild() { public void shouldReturnAddedChild() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
val child = new NodeItem<String>("child", Node::getData); val child = new NodeItem<String>("child");
//when //when
node.addChild(child); node.addChild(child);
//then //then
@ -252,7 +252,7 @@ public class NodeItemTest {
@Test @Test
public void addChildShouldThrowNodeExceptionWhenAddingANodeAsOwnChild() { public void addChildShouldThrowNodeExceptionWhenAddingANodeAsOwnChild() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
exception.expect(NodeException.class); exception.expect(NodeException.class);
exception.expectMessage("Child is an ancestor"); exception.expectMessage("Child is an ancestor");
//then //then
@ -265,7 +265,7 @@ public class NodeItemTest {
@Test @Test
public void addChildShouldThrowNodeExceptionWhenAddingSelfAsChild() { public void addChildShouldThrowNodeExceptionWhenAddingSelfAsChild() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
exception.expect(NodeException.class); exception.expect(NodeException.class);
exception.expectMessage("Child is an ancestor"); exception.expectMessage("Child is an ancestor");
//when //when
@ -279,7 +279,7 @@ public class NodeItemTest {
@Test @Test
public void addChildShouldThrowNodeExceptionWhenChildIsParent() { public void addChildShouldThrowNodeExceptionWhenChildIsParent() {
//given //given
val parent = new NodeItem<String>("parent", Node::getData); val parent = new NodeItem<String>("parent");
node = new NodeItem<>("subject", parent); node = new NodeItem<>("subject", parent);
exception.expect(NodeException.class); exception.expect(NodeException.class);
exception.expectMessage("Child is an ancestor"); exception.expectMessage("Child is an ancestor");
@ -294,7 +294,7 @@ public class NodeItemTest {
@Test @Test
public void addChildShouldThrowNodeExceptionWhenAddingGrandParentAsChild() { public void addChildShouldThrowNodeExceptionWhenAddingGrandParentAsChild() {
//given //given
val grandParent = new NodeItem<String>("grandparent", Node::getData); val grandParent = new NodeItem<String>("grandparent");
val parent = new NodeItem<String>("parent", grandParent); val parent = new NodeItem<String>("parent", grandParent);
node = new NodeItem<>("subject", parent); node = new NodeItem<>("subject", parent);
exception.expect(NodeException.class); exception.expect(NodeException.class);
@ -309,8 +309,8 @@ public class NodeItemTest {
@Test @Test
public void shouldSetParentOnChildWhenAddedAsChild() { public void shouldSetParentOnChildWhenAddedAsChild() {
//given //given
val child = new NodeItem<String>("child", Node::getData); val child = new NodeItem<String>("child");
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
//when //when
node.addChild(child); node.addChild(child);
//then //then
@ -326,7 +326,7 @@ public class NodeItemTest {
public void shouldWalkTreeToNode() { public void shouldWalkTreeToNode() {
//given //given
val grandparent = "grandparent"; val grandparent = "grandparent";
val grandParentNode = new NodeItem<String>(grandparent, Node::getData); val grandParentNode = new NodeItem<String>(grandparent);
val parent = "parent"; val parent = "parent";
val parentNode = new NodeItem<String>(parent, grandParentNode); val parentNode = new NodeItem<String>(parent, grandParentNode);
val subject = "subject"; val subject = "subject";
@ -351,7 +351,7 @@ public class NodeItemTest {
public void shouldNotFindNonExistentChildNode() { public void shouldNotFindNonExistentChildNode() {
//given //given
val parent = "parent"; val parent = "parent";
val parentNode = new NodeItem<String>(parent, Node::getData); val parentNode = new NodeItem<String>(parent);
val subject = "subject"; val subject = "subject";
node = new NodeItem<>(subject, parentNode); node = new NodeItem<>(subject, parentNode);
//when //when
@ -368,7 +368,7 @@ public class NodeItemTest {
@Test @Test
public void shouldThrowNEWhenWalkTreeNull() { public void shouldThrowNEWhenWalkTreeNull() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
exception.expect(NullPointerException.class); exception.expect(NullPointerException.class);
exception.expectMessage("path"); exception.expectMessage("path");
//when //when
@ -382,7 +382,7 @@ public class NodeItemTest {
@Test @Test
public void shouldReturnEmptyForEmptyWalkTreePath() { public void shouldReturnEmptyForEmptyWalkTreePath() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
//when //when
val result = node.findInPath(Collections.emptyList()); val result = node.findInPath(Collections.emptyList());
//then //then
@ -395,7 +395,7 @@ public class NodeItemTest {
@Test @Test
public void shouldCreateDescendantNodes() { public void shouldCreateDescendantNodes() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
val alphaData = "alpha"; val alphaData = "alpha";
val betaData = "beta"; val betaData = "beta";
val gammaData = "gamma"; val gammaData = "gamma";
@ -444,7 +444,7 @@ public class NodeItemTest {
@Test @Test
public void createDescendantLineShouldThrowNPEWhenDescendantsAreNull() { public void createDescendantLineShouldThrowNPEWhenDescendantsAreNull() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
exception.expect(NullPointerException.class); exception.expect(NullPointerException.class);
exception.expectMessage("descendants"); exception.expectMessage("descendants");
//when //when
@ -457,7 +457,7 @@ public class NodeItemTest {
@Test @Test
public void shouldChangeNothingWhenCreateDescendantEmpty() { public void shouldChangeNothingWhenCreateDescendantEmpty() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
//when //when
node.createDescendantLine(Collections.emptyList()); node.createDescendantLine(Collections.emptyList());
//then //then
@ -472,7 +472,7 @@ public class NodeItemTest {
@Test @Test
public void shouldFindExistingChildNode() { public void shouldFindExistingChildNode() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
val childData = "child"; val childData = "child";
val child = new NodeItem<String>(childData, node); val child = new NodeItem<String>(childData, node);
//when //when
@ -489,14 +489,14 @@ public class NodeItemTest {
@Test @Test
public void shouldFindCreateNewChildNode() { public void shouldFindCreateNewChildNode() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
val childData = "child"; val childData = "child";
//when //when
val found = node.findOrCreateChild(childData); val found = node.findOrCreateChild(childData);
//then //then
assertThat(found.getData()).as( assertThat(found.getData()).as(
"when searching for a child by data, a new node is created") "when searching for a non-existent child by data, a new node "
.isSameAs(childData); + "is created").contains(childData);
} }
/** /**
@ -505,7 +505,7 @@ public class NodeItemTest {
@Test @Test
public void findOrCreateChildShouldThrowNPEFWhenChildIsNull() { public void findOrCreateChildShouldThrowNPEFWhenChildIsNull() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
exception.expect(NullPointerException.class); exception.expect(NullPointerException.class);
exception.expectMessage("child"); exception.expectMessage("child");
//when //when
@ -518,9 +518,9 @@ public class NodeItemTest {
@Test @Test
public void shouldGetChild() { public void shouldGetChild() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
val childData = "child"; val childData = "child";
val child = new NodeItem<String>(childData, Node::getData); val child = new NodeItem<String>(childData);
node.addChild(child); node.addChild(child);
//when //when
val found = node.findChild(childData); val found = node.findChild(childData);
@ -540,7 +540,7 @@ public class NodeItemTest {
@Test @Test
public void getChildShouldThrowNPEWhenThereIsNoChild() { public void getChildShouldThrowNPEWhenThereIsNoChild() {
//given //given
node = new NodeItem<>("data", Node::getData); node = new NodeItem<>("data");
exception.expect(NullPointerException.class); exception.expect(NullPointerException.class);
exception.expectMessage("child"); exception.expectMessage("child");
//when //when
@ -554,7 +554,7 @@ public class NodeItemTest {
@Test @Test
public void shouldCreateChild() { public void shouldCreateChild() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
val childData = "child"; val childData = "child";
//when //when
val child = node.createChild(childData); val child = node.createChild(childData);
@ -579,7 +579,7 @@ public class NodeItemTest {
@Test @Test
public void createChildShouldThrowNPEWhenChildIsNull() { public void createChildShouldThrowNPEWhenChildIsNull() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject");
exception.expect(NullPointerException.class); exception.expect(NullPointerException.class);
exception.expectMessage("child"); exception.expectMessage("child");
//when //when
@ -589,7 +589,7 @@ public class NodeItemTest {
@Test @Test
public void getNameShouldBeCorrect() { public void getNameShouldBeCorrect() {
//given //given
node = new NodeItem<>("subject", Node::getData); node = new NodeItem<>("subject", n -> n.getData().get());
//then //then
assertThat(node.getName()).isEqualTo("subject"); assertThat(node.getName()).isEqualTo("subject");
} }
@ -597,7 +597,7 @@ public class NodeItemTest {
@Test @Test
public void getNameShouldUseParentNameSupplier() { public void getNameShouldUseParentNameSupplier() {
//given //given
val root = new NodeItem<String>("root", Node::getData); val root = new NodeItem<String>("root", n -> n.getData().get());
node = new NodeItem<>("child", root); node = new NodeItem<>("child", root);
//then //then
assertThat(node.getName()).isEqualTo("child"); assertThat(node.getName()).isEqualTo("child");
@ -605,15 +605,20 @@ public class NodeItemTest {
@Test @Test
public void getNameShouldReturnNameForNonStringData() { public void getNameShouldReturnNameForNonStringData() {
val root = new NodeItem<LocalDate>(LocalDate.parse("2016-05-23"), val root = new NodeItem<LocalDate>(LocalDate.parse("2016-05-23"), n -> {
n -> n.getData().format(DateTimeFormatter.BASIC_ISO_DATE)); if (n.isEmpty()) {
return null;
}
return n.getData().get().format(DateTimeFormatter.BASIC_ISO_DATE);
});
//then //then
assertThat(root.getName()).isEqualTo("20160523"); assertThat(root.getName()).isEqualTo("20160523");
} }
@Test @Test
public void getNameShouldUseClosestNameSupplier() { public void getNameShouldUseClosestNameSupplier() {
node = new NodeItem<>("root", Node::getData); node = new NodeItem<>("root", n -> n.getData().get());
val child = new NodeItem<String>("child", Object::toString); val child = new NodeItem<String>("child", Object::toString);
node.addChild(child); node.addChild(child);
val grandChild = new NodeItem<>("grandchild", child); val grandChild = new NodeItem<>("grandchild", child);
@ -635,13 +640,13 @@ public class NodeItemTest {
@Test @Test
public void canCreateRootNodeWithoutData() { public void canCreateRootNodeWithoutData() {
node = new NodeItem<>(null, "empty"); node = new NodeItem<>(null, "empty");
assertThat(node.getData()).isNull(); assertThat(node.getData()).isEmpty();
} }
@Test @Test
public void canCreateRootNodeWithoutDataButWithNameSupplier() { public void canCreateRootNodeWithoutDataButWithNameSupplier() {
node = new NodeItem<>(null, Node::getData); node = new NodeItem<>(null);
assertThat(node.getData()).isNull(); assertThat(node.getData()).isEmpty();
} }
@Test @Test
@ -736,7 +741,7 @@ public class NodeItemTest {
node.insertInPath(child); node.insertInPath(child);
//then //then
assertThat(node.getChildByName("child").getData()).as("data in tree") assertThat(node.getChildByName("child").getData()).as("data in tree")
.isSameAs( .contains(
"child data"); "child data");
assertThat( assertThat(
node.getChildByName("child").getChildByName("grandchild")).as( node.getChildByName("child").getChildByName("grandchild")).as(
@ -808,7 +813,7 @@ public class NodeItemTest {
node.insertInPath(addMe, "child"); node.insertInPath(addMe, "child");
//then //then
assertThat(child.getChildByName("target").getData()).as( assertThat(child.getChildByName("target").getData()).as(
"target now contains data").isEqualTo("I'm new"); "target now contains data").contains("I'm new");
} }
@Test @Test
@ -850,7 +855,7 @@ public class NodeItemTest {
// once a node has it's parent removed it should provide a default name // once a node has it's parent removed it should provide a default name
// provider // provider
//given //given
node = new NodeItem<>("data", Node::getData); // name provider: getData node = new NodeItem<>("data", n -> n.getData().get());
final NodeItem<String> child = new NodeItem<>("other", node); final NodeItem<String> child = new NodeItem<>("other", node);
assertThat(node.getName()).as("initial root name").isEqualTo("data"); assertThat(node.getName()).as("initial root name").isEqualTo("data");
assertThat(child.getName()).as("initial child name").isEqualTo("other"); assertThat(child.getName()).as("initial child name").isEqualTo("other");
@ -903,7 +908,7 @@ public class NodeItemTest {
//when //when
node.setData("updated"); node.setData("updated");
//then //then
assertThat(node.getData()).isEqualTo("updated"); assertThat(node.getData()).contains("updated");
} }
@Test @Test
@ -946,7 +951,7 @@ public class NodeItemTest {
//given //given
node = new NodeItem<>(null); node = new NodeItem<>(null);
//when //when
NodeItem<String> child = new NodeItem<>(null, Node::getData, node); NodeItem<String> child = new NodeItem<>(null, node);
//then //then
assertThat(child.getParent()).isSameAs(node); assertThat(child.getParent()).isSameAs(node);
assertThat(node.getChildren()).containsExactly(child); assertThat(node.getChildren()).containsExactly(child);
@ -1023,7 +1028,7 @@ public class NodeItemTest {
if (parent == null) { if (parent == null) {
return ""; return "";
} }
return parent.getName() + "/" + node.getData(); return parent.getName() + "/" + node.getData().get();
}; };
node = new NodeItem<>(null, pathNameSupplier); node = new NodeItem<>(null, pathNameSupplier);
val child = new NodeItem<String>("child", node); val child = new NodeItem<String>("child", node);
@ -1031,4 +1036,13 @@ public class NodeItemTest {
//then //then
assertThat(grandchild.getName()).isEqualTo("/child/grandchild"); assertThat(grandchild.getName()).isEqualTo("/child/grandchild");
} }
@Test
public void canSafelyHandleFindChildWhenAChildHasNoData() {
//given
node = new NodeItem<>(null);
new NodeItem<>(null, node);
//when
node.findChild("data");
}
} }