From 9fec49cc58e7cded72bf9a75156b0c8aed55e874 Mon Sep 17 00:00:00 2001
From: Andreas Troelsen <garbagemule@gmail.com>
Date: Sat, 7 Aug 2021 01:25:30 +0200
Subject: [PATCH] Make ThingGroupPicker aware of result set size.

This commit makes the ThingGroupPicker return different values depending
on the size of the picked result set. If the set is empty, the picker
returns null, emulating a NothingPicker. If the set is a singleton, the
element itself is returned, emulating the SingleThingPicker. Finally, if
the set contains more than one element, a ThingGroup containing those
elements is returned.
---
 .../MobArena/things/ThingGroupPicker.java     |  8 ++
 .../MobArena/things/ThingGroupPickerTest.java | 83 +++++++++++++++----
 2 files changed, 77 insertions(+), 14 deletions(-)

diff --git a/src/main/java/com/garbagemule/MobArena/things/ThingGroupPicker.java b/src/main/java/com/garbagemule/MobArena/things/ThingGroupPicker.java
index 8506b1c..4266f0d 100644
--- a/src/main/java/com/garbagemule/MobArena/things/ThingGroupPicker.java
+++ b/src/main/java/com/garbagemule/MobArena/things/ThingGroupPicker.java
@@ -19,6 +19,14 @@ public class ThingGroupPicker implements ThingPicker {
             .filter(Objects::nonNull)
             .collect(Collectors.toList());
 
+        if (things.isEmpty()) {
+            return null;
+        }
+
+        if (things.size() == 1) {
+            return things.get(0);
+        }
+
         return new ThingGroup(things);
     }
 
diff --git a/src/test/java/com/garbagemule/MobArena/things/ThingGroupPickerTest.java b/src/test/java/com/garbagemule/MobArena/things/ThingGroupPickerTest.java
index ae1affb..56dc735 100644
--- a/src/test/java/com/garbagemule/MobArena/things/ThingGroupPickerTest.java
+++ b/src/test/java/com/garbagemule/MobArena/things/ThingGroupPickerTest.java
@@ -2,14 +2,17 @@ package com.garbagemule.MobArena.things;
 
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.junit.MockitoJUnitRunner;
 
 import java.util.ArrayList;
 import java.util.List;
 
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
+import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.MatcherAssert.*;
+import static org.mockito.Mockito.*;
 
+@RunWith(MockitoJUnitRunner.StrictStubs.class)
 public class ThingGroupPickerTest {
 
     private ThingGroupPicker subject;
@@ -22,19 +25,71 @@ public class ThingGroupPickerTest {
     }
 
     @Test
-    public void invokesAllPickers() {
-        ThingPicker first = mock(ThingPicker.class);
-        ThingPicker second = mock(ThingPicker.class);
-        ThingPicker third = mock(ThingPicker.class);
-        pickers.add(first);
-        pickers.add(second);
-        pickers.add(third);
+    public void returnsNullIfResultIsEmpty() {
+        Thing result = subject.pick();
 
-        subject.pick();
+        assertThat(result, nullValue());
+    }
 
-        verify(first, times(1)).pick();
-        verify(second, times(1)).pick();
-        verify(third, times(1)).pick();
+    @Test
+    public void returnsSingletonItemIfResultSizeIsOne() {
+        Thing thing = mock(Thing.class);
+        ThingPicker picker = mock(ThingPicker.class);
+        when(picker.pick()).thenReturn(thing);
+        pickers.add(picker);
+
+        Thing result = subject.pick();
+
+        assertThat(result, sameInstance(thing));
+    }
+
+    @Test
+    public void returnsThingGroupIfResultSizeIsGreaterThanOne() {
+        Thing thing1 = mock(Thing.class);
+        Thing thing2 = mock(Thing.class);
+        ThingPicker picker1 = mock(ThingPicker.class);
+        ThingPicker picker2 = mock(ThingPicker.class);
+        when(picker1.pick()).thenReturn(thing1);
+        when(picker2.pick()).thenReturn(thing2);
+        pickers.add(picker1);
+        pickers.add(picker2);
+
+        Thing result = subject.pick();
+
+        assertThat(result, instanceOf(ThingGroup.class));
+    }
+
+    @Test
+    public void skipsNullPickInTwoPickersAndReturnsSingleton() {
+        // This test is kind of "cheating" in that we're building off
+        // the knowledge that a singleton result is returned as-is,
+        // but at the time of writing, there's no straightforward way
+        // to verify that nulls are omitted.
+        Thing thing2 = mock(Thing.class);
+        ThingPicker picker1 = mock(ThingPicker.class);
+        ThingPicker picker2 = mock(ThingPicker.class);
+        when(picker1.pick()).thenReturn(null);
+        when(picker2.pick()).thenReturn(thing2);
+        pickers.add(picker1);
+        pickers.add(picker2);
+
+        Thing result = subject.pick();
+
+        assertThat(result, sameInstance(thing2));
+    }
+
+    @Test
+    public void toStringConcatenatesWithAnd() {
+        ThingPicker picker1 = mock(ThingPicker.class);
+        ThingPicker picker2 = mock(ThingPicker.class);
+        when(picker1.toString()).thenReturn("p1");
+        when(picker2.toString()).thenReturn("p2");
+        pickers.add(picker1);
+        pickers.add(picker2);
+
+        String result = subject.toString();
+
+        assertThat(result, equalTo("(p1 and p2)"));
     }
 
 }