Using Optionals in BGG and removing NoItemsFoundException

marcioos · Oct 28, 2016 · 3817e49 · 3817e49
1 parent 2849c5b
commit 3817e49
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 77 deletions.
diff --git a/src/main/java/co/yellowbricks/bggclient/BGG.java b/src/main/java/co/yellowbricks/bggclient/BGG.java
@@ -1,54 +1,62 @@
 package co.yellowbricks.bggclient;
 
-import co.yellowbricks.bggclient.common.NoItemsFoundException;
 import co.yellowbricks.bggclient.common.ThingType;
 import co.yellowbricks.bggclient.fetch.FetchException;
 import co.yellowbricks.bggclient.fetch.domain.FetchItem;
 import co.yellowbricks.bggclient.fetch.domain.FetchItemOutput;
 import co.yellowbricks.bggclient.fetch.domain.UserCollection;
 import co.yellowbricks.bggclient.search.SearchException;
 import co.yellowbricks.bggclient.search.domain.SearchOutput;
+import retrofit2.Response;
 
 import java.util.Collection;
+import java.util.Optional;
+
+import static java.util.Collections.emptyList;
+import static java.util.Optional.empty;
+import static java.util.Optional.ofNullable;
 
 public final class BGG {
 
     private BGG() {
     }
 
-    public static SearchOutput search(String query, ThingType... thingTypes) throws SearchException, NoItemsFoundException {
+    public static Optional<SearchOutput> search(String query, ThingType... thingTypes) throws SearchException {
         try {
-            SearchOutput items =
-                    BggService.INSTANCE.search(query, getTypesQueryString(thingTypes)).execute().body();
+            Response<SearchOutput> searchResponse =
+                    BggService.INSTANCE.search(query, getTypesQueryString(thingTypes)).execute();
 
-            if (items.getItems() != null && !items.getItems().isEmpty())
-                return items;
-            throw new NoItemsFoundException();
+            if (searchResponse.isSuccessful()) {
+                return ofNullable(searchResponse.body());
+            }
+            return empty();
         } catch (Exception e) {
             throw new SearchException("While searching for " + query, e);
         }
     }
 
-    public static Collection<FetchItem> fetch(Collection<Integer> ids, ThingType... thingTypes) throws FetchException, NoItemsFoundException {
+    public static Collection<FetchItem> fetch(Collection<Integer> ids, ThingType... thingTypes) throws FetchException {
         try {
-            FetchItemOutput items =
-                    BggService.INSTANCE.fetch(getIdsAsString(ids), getTypesQueryString(thingTypes)).execute().body();
+            Response<FetchItemOutput> fetchResponse =
+                    BggService.INSTANCE.fetch(getIdsAsString(ids), getTypesQueryString(thingTypes)).execute();
 
-            if (items.getItems() != null && !items.getItems().isEmpty())
-                return items.getItems();
-            throw new NoItemsFoundException();
+            if (fetchResponse.isSuccessful()) {
+                return fetchResponse.body().getItems();
+            }
+            return emptyList();
         } catch (Exception e) {
             throw new FetchException("While fetching ids: " + ids, e);
         }
     }
 
-    public static UserCollection fetchCollection(String ownerName) throws FetchException, NoItemsFoundException {
+    public static Optional<UserCollection> fetchCollection(String ownerName) throws FetchException {
         try {
-            UserCollection collection = BggService.INSTANCE.fetchCollection(ownerName, 1).execute().body();
+            Response<UserCollection> collectionResponse = BggService.INSTANCE.fetchCollection(ownerName, 1).execute();
 
-            if (collection.getItems() != null && !collection.getItems().isEmpty())
-                return collection;
-            throw new NoItemsFoundException();
+            if (collectionResponse.isSuccessful()) {
+                return ofNullable(collectionResponse.body());
+            }
+            return empty();
         } catch (Exception e) {
             throw new FetchException("While fetching %s's collection " + ownerName, e);
         }

diff --git a/src/main/java/co/yellowbricks/bggclient/common/NoItemsFoundException.java b/src/main/java/co/yellowbricks/bggclient/common/NoItemsFoundException.java
diff --git a/src/main/java/co/yellowbricks/bggclient/fetch/domain/FetchItemOutput.java b/src/main/java/co/yellowbricks/bggclient/fetch/domain/FetchItemOutput.java
@@ -1,12 +1,15 @@
 package co.yellowbricks.bggclient.fetch.domain;
 
-import java.util.List;
-import java.util.Objects;
-
 import org.simpleframework.xml.Attribute;
 import org.simpleframework.xml.ElementList;
 import org.simpleframework.xml.Root;
 
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+import static java.util.Optional.ofNullable;
+
 @Root(name = "items", strict = false)
 public class FetchItemOutput {
 
@@ -21,7 +24,7 @@ public String getTermsOfUseUrl() {
     }
 
 	public List<FetchItem> getItems() {
-        return items;
+        return ofNullable(items).orElseGet(Collections::emptyList);
     }
 
     @Override

diff --git a/src/test/java/co/yellowbricks/bggclient/BGGFetchTest.java b/src/test/java/co/yellowbricks/bggclient/BGGFetchTest.java
@@ -1,29 +1,26 @@
 package co.yellowbricks.bggclient;
 
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.not;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-
-import org.hamcrest.CoreMatchers;
-import org.junit.Test;
-
-import co.yellowbricks.bggclient.common.NoItemsFoundException;
 import co.yellowbricks.bggclient.common.ThingType;
 import co.yellowbricks.bggclient.fetch.FetchException;
 import co.yellowbricks.bggclient.fetch.domain.CollectionItem;
 import co.yellowbricks.bggclient.fetch.domain.FetchItem;
 import co.yellowbricks.bggclient.fetch.domain.UserCollection;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 
 public class BGGFetchTest {
 
     @Test
-    public void shouldFetchDieMacher() throws FetchException, NoItemsFoundException {
+    public void shouldFetchDieMacher() throws FetchException {
         int dieMacherId = 1;
 
         FetchItem item = BGG.fetch(Arrays.asList(dieMacherId), ThingType.BOARDGAME).iterator().next();
@@ -32,7 +29,7 @@ public void shouldFetchDieMacher() throws FetchException, NoItemsFoundException
     }
 
     @Test
-    public void shouldFetchAgricolaXDeck() throws FetchException, NoItemsFoundException {
+    public void shouldFetchAgricolaXDeck() throws FetchException {
         int agricolaXDeckId = 38733;
 
         FetchItem item = BGG.fetch(Arrays.asList(agricolaXDeckId), ThingType.BOARDGAME_EXPANSION).iterator().next();
@@ -41,7 +38,7 @@ public void shouldFetchAgricolaXDeck() throws FetchException, NoItemsFoundExcept
     }
 
     @Test
-    public void shouldFetchAgricolaXDeckAndDieMacher() throws FetchException, NoItemsFoundException {
+    public void shouldFetchAgricolaXDeckAndDieMacher() throws FetchException {
         int agricolaXDeckId = 38733;
         int dieMacherId = 1;
 
@@ -52,42 +49,35 @@ public void shouldFetchAgricolaXDeckAndDieMacher() throws FetchException, NoItem
     }
 
     @Test(expected = FetchException.class)
-    public void shouldFindNoItems() throws FetchException, NoItemsFoundException {
+    public void shouldFindNoItems() throws FetchException {
         BGG.fetch(Arrays.asList(13123123, 2380182));
     }
 
     @Test
-    public void shouldFetchMyCollection() throws FetchException, NoItemsFoundException {
+    public void shouldFetchMyCollection() throws FetchException {
         String myName = "marcio_os";
 
-        UserCollection myCollection = BGG.fetchCollection(myName);
+        UserCollection myCollection = BGG.fetchCollection(myName).get();
 
         assertThat(myCollection.getTotalItems(), is(not(0)));
     }
 
     @Test
-    public void collectionShouldContainBGGTermsOfUseUrl() throws FetchException, NoItemsFoundException {
+    public void collectionShouldContainBGGTermsOfUseUrl() throws FetchException {
         String myName = "marcio_os";
 
-        UserCollection myCollection = BGG.fetchCollection(myName);
+        UserCollection myCollection = BGG.fetchCollection(myName).get();
 
         assertThat(myCollection.getTermsOfUseUrl(), is("http://boardgamegeek.com/xmlapi/termsofuse"));
     }
 
     @Test
-    public void myCollectionShouldContainDominion() throws FetchException, NoItemsFoundException {
+    public void myCollectionShouldContainDominion() throws FetchException {
         String myName = "marcio_os";
 
-        UserCollection myCollection = BGG.fetchCollection(myName);
+        UserCollection myCollection = BGG.fetchCollection(myName).get();
 
         for (CollectionItem item : myCollection.getItems()) if ("Dominion".equals(item.getName())) return;
         fail("Dominion not found");
     }
-
-    @Test
-    public void fetchBevatronCollection() throws FetchException, NoItemsFoundException {
-        String userName = "bevatron";
-
-        UserCollection collection = BGG.fetchCollection(userName);
-    }
 }
diff --git a/src/test/java/co/yellowbricks/bggclient/BGGSearchTest.java b/src/test/java/co/yellowbricks/bggclient/BGGSearchTest.java
@@ -1,6 +1,5 @@
 package co.yellowbricks.bggclient;
 
-import co.yellowbricks.bggclient.common.NoItemsFoundException;
 import co.yellowbricks.bggclient.common.ThingType;
 import co.yellowbricks.bggclient.search.SearchException;
 import co.yellowbricks.bggclient.search.domain.SearchItem;
@@ -9,31 +8,36 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+
 public class BGGSearchTest {
 
     @Test
-    public void shouldReturnCorrectAmountOfDominionGames() throws SearchException, NoItemsFoundException {
-        SearchOutput items = BGG.search("dominion", ThingType.BOARDGAME);
+    public void shouldReturnCorrectAmountOfDominionGames() throws SearchException {
+        SearchOutput items = BGG.search("dominion", ThingType.BOARDGAME).get();
 
-        Assert.assertThat(items.getTotal(), CoreMatchers.is(52));
+        assertThat(items.getTotal(), is(52));
     }
 
-    @Test(expected = SearchException.class)
-    public void shouldFindNoItems() throws SearchException, NoItemsFoundException {
-        BGG.search("a game that should not exist");
+    public void shouldFindNoItems() throws SearchException {
+        assertThat(BGG.search("a game that should not exist"), is(Optional.empty()));
     }
 
     @Test
-    public void searchBoardgamesShouldReturnExpansion() throws SearchException, NoItemsFoundException {
+    public void searchBoardgamesShouldReturnExpansion() throws SearchException {
         int agricolaXDeckId = 38733;
 
-        SearchOutput items = BGG.search("agricola", ThingType.BOARDGAME);
+        SearchOutput items = BGG.search("agricola", ThingType.BOARDGAME).get();
 
         for (SearchItem item : items.getItems()) {
             if (item.getId() == agricolaXDeckId) {
                 return;
             }
         }
-        Assert.fail("SearchOutput does not contain Agricola X-Deck id");
+        fail("SearchOutput does not contain Agricola X-Deck id");
     }
 }
diff --git a/src/test/java/co/yellowbricks/bggclient/fetch/domain/FetchItemTest.java b/src/test/java/co/yellowbricks/bggclient/fetch/domain/FetchItemTest.java
@@ -1,50 +1,48 @@
 package co.yellowbricks.bggclient.fetch.domain;
 
-import java.util.Arrays;
-
+import co.yellowbricks.bggclient.BGG;
+import co.yellowbricks.bggclient.fetch.FetchException;
 import org.hamcrest.CoreMatchers;
 import org.junit.Assert;
 import org.junit.Test;
 
-import co.yellowbricks.bggclient.BGG;
-import co.yellowbricks.bggclient.common.NoItemsFoundException;
-import co.yellowbricks.bggclient.fetch.FetchException;
+import java.util.Arrays;
 
 public class FetchItemTest {
 
     private static final int DIE_MACHER_ID = 1;
     private static final int DOMINION_2004_ID = 64777;
 
     @Test
-    public void shouldGetBestNumberOfPlayersForDieMacher() throws FetchException, NoItemsFoundException {
+    public void shouldGetBestNumberOfPlayersForDieMacher() throws FetchException {
         FetchItem dieMacher = BGG.fetch(Arrays.asList(DIE_MACHER_ID)).iterator().next();
 
         Assert.assertThat(dieMacher.getBestNumberOfPlayers(), CoreMatchers.equalTo("5"));
     }
 
     @Test
-    public void shouldGetDieMacherCategories() throws FetchException, NoItemsFoundException {
+    public void shouldGetDieMacherCategories() throws FetchException {
         FetchItem dieMacher = BGG.fetch(Arrays.asList(DIE_MACHER_ID)).iterator().next();
 
         Assert.assertThat(dieMacher.getCategories(), CoreMatchers.equalTo(Arrays.asList("Economic", "Negotiation", "Political")));
     }
 
     @Test
-    public void shouldGetDieMacherMechanics() throws FetchException, NoItemsFoundException {
+    public void shouldGetDieMacherMechanics() throws FetchException {
         FetchItem dieMacher = BGG.fetch(Arrays.asList(DIE_MACHER_ID)).iterator().next();
 
         Assert.assertThat(dieMacher.getMechanics(), CoreMatchers.equalTo(Arrays.asList("Area Control / Area Influence", "Auction/Bidding", "Dice Rolling", "Hand Management", "Simultaneous Action Selection")));
     }
 
     @Test
-    public void shouldGetDieMacherDesigners() throws FetchException, NoItemsFoundException {
+    public void shouldGetDieMacherDesigners() throws FetchException {
         FetchItem dieMacher = BGG.fetch(Arrays.asList(DIE_MACHER_ID)).iterator().next();
 
         Assert.assertThat(dieMacher.getDesigners(), CoreMatchers.equalTo(Arrays.asList("Karl-Heinz Schmiel")));
     }
 
     @Test
-    public void dominion2004ShouldHaveUnknownBestNumberOfPlayers() throws FetchException, NoItemsFoundException {
+    public void dominion2004ShouldHaveUnknownBestNumberOfPlayers() throws FetchException {
         FetchItem dominion2004 = BGG.fetch(Arrays.asList(DOMINION_2004_ID)).iterator().next();
 
         Assert.assertThat(dominion2004.getBestNumberOfPlayers(), CoreMatchers.equalTo("unknown"));