Refactored comparators, did a little bit of CLI testing but will need to look into mocking

5 files changed, 35 insertions, 38 deletions

diff --git a/src/eu/equalparts/cardbase/cli/CardbaseCLI.java b/src/eu/equalparts/cardbase/cli/CardbaseCLI.java
index 0b2c869..8c0c97a 100644
--- a/src/eu/equalparts/cardbase/cli/CardbaseCLI.java
+++ b/src/eu/equalparts/cardbase/cli/CardbaseCLI.java
@@ -48,6 +48,10 @@ public final class CardbaseCLI {
 	 */
 	Action lastAction = null;
 	/**
+	 * 
+	 */
+	MTGUniverse mtgUniverse = new MTGUniverse();
+	/**
 	 * The currently selected set, from which new cards are added.
 	 */
 	FullCardSet selectedSet = null;
@@ -267,7 +271,7 @@ public final class CardbaseCLI {
 	 * Print a list of valid set codes.
 	 */
 	void sets() {
-		for (CardSetInformation set : MTGUniverse.getCardSetList()) {
+		for (CardSetInformation set : mtgUniverse.getCardSetList()) {
 			// CardSet has an overridden toString()
 			System.out.println(set);
 		}
@@ -281,7 +285,7 @@ public final class CardbaseCLI {
 	void set(String... args) {
 		if (args != null && args.length > 0) {
 			try {
-				selectedSet = MTGUniverse.getFullCardSet(args[0]);
+				selectedSet = mtgUniverse.getFullCardSet(args[0]);
 				// if the set code is invalid, null is returned
 				if (selectedSet != null) {
 					System.out.println("Selected set: " + selectedSet.name + ".");
diff --git a/src/eu/equalparts/cardbase/comparator/CardComparator.java b/src/eu/equalparts/cardbase/comparator/CardComparator.java
index 9f90e00..7173109 100644
--- a/src/eu/equalparts/cardbase/comparator/CardComparator.java
+++ b/src/eu/equalparts/cardbase/comparator/CardComparator.java
@@ -37,7 +37,7 @@ public class CardComparator implements Comparator<Card> {
 	/**
 	 * The comparison delegate to use for the specified field.
 	 */
-	private BiFunction<Comparable, Comparable, Integer> selectedComparisonDelegate = this::defaultComparison;
+	private BiFunction<Comparable, Comparable, Integer> comparisonDelegate = (field1, field2) -> field1.compareTo(field2);
 	
 	/**
 	 * Creates a new comparator for the specified field only. This class
@@ -58,9 +58,9 @@ public class CardComparator implements Comparator<Card> {
 			// if annotated with a special comparator, set the comparison delegate here
 			for (Annotation annotation : fieldToCompare.getAnnotations()) {
 				if (annotation.annotationType() == DirtyNumber.class) {
-					this.selectedComparisonDelegate = ComparatorDelegates::compareDirtyNumber;
+					this.comparisonDelegate = ComparatorDelegates::compareDirtyNumber;
 				} else if (annotation.annotationType() == Rarity.class) {
-					this.selectedComparisonDelegate = ComparatorDelegates::compareRarity;
+					this.comparisonDelegate = ComparatorDelegates::compareRarity;
 				}
 			}
 		} else {
@@ -72,7 +72,7 @@ public class CardComparator implements Comparator<Card> {
 	public int compare(Card o1, Card o2) {
 		/*
 		 * we've already checked that the field is self comparable,
-		 * so we are now free to cast to whatever type it is and compare.
+		 * so we are now free to cast to comparable
 		 */
 		try {
 			Comparable field1 = (Comparable) fieldToCompare.get(o1);
@@ -88,7 +88,7 @@ public class CardComparator implements Comparator<Card> {
 			} else if (field2 == null) {
 				return 1;
 			} else {
-				return selectedComparisonDelegate.apply(field1, field2);
+				return comparisonDelegate.apply(field1, field2);
 			}
 			
 		} catch (IllegalArgumentException e) {
@@ -102,20 +102,6 @@ public class CardComparator implements Comparator<Card> {
 		// fallback, this shouldn't happen
 		return 0;
 	}
-	
-	/**
-	 * The standard comparison operation, which uses the field's own {@code compareTo()}
-	 * method.
-	 * 
-	 * @param field1 the first object to be compared.
-     * @param field2 the second object to be compared.
-     * @return a negative integer, zero, or a positive integer as the
-     *         first argument is less than, equal to, or greater than the
-     *         second.
-	 */
-	private int defaultComparison(Comparable field1, Comparable field2) {
-		return field1.compareTo(field2);
-	}
 
 	/**
 	 * Use reflection to determine if the specified class can be compared with itself.
diff --git a/src/eu/equalparts/cardbase/comparator/ComparatorDelegates.java b/src/eu/equalparts/cardbase/comparator/ComparatorDelegates.java
index 7bf09ac..cede956 100644
--- a/src/eu/equalparts/cardbase/comparator/ComparatorDelegates.java
+++ b/src/eu/equalparts/cardbase/comparator/ComparatorDelegates.java
@@ -8,6 +8,17 @@ final class ComparatorDelegates {
 	/*
 	 * Delegates
 	 */
+	/**
+	 * Compares two dirty numbers, that is, strings containing a single number
+	 * and, optionally, additional non-numerical characters.
+	 * For instance, when comparing "150a" and "36", this comparator delegate
+	 * first compares 150 with 36 using the standard integer comparison, and
+	 * returns an integer > 1 because 150 > 36.
+	 * 
+	 * @param field1 the first field to compare.
+	 * @param field2 the second field to compare.
+	 * @return a value smaller than, equal to or greater than 0 according to the standard comparison convention.
+	 */
 	public static Integer compareDirtyNumber(Comparable<String> field1, Comparable<String> field2) {
 		// this assumes that the format is uninterrupted numbers and letters
 		String number1 = ((String) field1).replaceAll("[^0-9]+", "");
diff --git a/src/eu/equalparts/cardbase/comparator/SpecialFields.java b/src/eu/equalparts/cardbase/comparator/SpecialFields.java
index f17ccc6..692f774 100644
--- a/src/eu/equalparts/cardbase/comparator/SpecialFields.java
+++ b/src/eu/equalparts/cardbase/comparator/SpecialFields.java
@@ -4,7 +4,9 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 
 
-public class SpecialFields {
+public final class SpecialFields {
+	
+	private SpecialFields() {}
 	
 	@Retention(RetentionPolicy.RUNTIME)
 	public @interface DirtyNumber {}
diff --git a/src/eu/equalparts/cardbase/utils/MTGUniverse.java b/src/eu/equalparts/cardbase/utils/MTGUniverse.java
index b40e518..98067fa 100644
--- a/src/eu/equalparts/cardbase/utils/MTGUniverse.java
+++ b/src/eu/equalparts/cardbase/utils/MTGUniverse.java
@@ -18,7 +18,7 @@ import eu.equalparts.cardbase.cards.FullCardSet;
 
 /**
  * Access point to the complete set of cards that exist in the
- * MTG universe. This class has a series of utility functions that
+ * MTG universe. This class has a series of methods that
  * query remote databases to acquire card information.
  * <br>
  * Conversely, {@code Cardbase}'s methods are used solely to
@@ -32,24 +32,19 @@ public final class MTGUniverse {
 	/**
 	 * The base URL from where the information is fetched.
 	 */
-	private static final String BASE_DATA_URL = "http://mtgjson.com/json/";
+	private final String BASE_DATA_URL = "http://mtgjson.com/json/";
 	/**
 	 * If the upstream set code list can't be loaded, this is loaded instead.
 	 */
-	private static final String FALLBACK_LIST_PATH = "/setlist.json";
+	private final String FALLBACK_LIST_PATH = "/setlist.json";
 	/**
 	 * A cache of CardSets to avoid querying the server many times for the same information.
 	 */
-	private static List<CardSetInformation> cardSets;
+	private List<CardSetInformation> cardSets;
 	/**
 	 * A cache of {@code FullCardSets} to avoid querying the server many times for the same information.
 	 */
-	private static HashMap<String, FullCardSet> cardSetCache = new HashMap<String, FullCardSet>();
-
-	/**
-	 * Private constructor, this class is not to be instantiated.
-	 */
-	private MTGUniverse() {}
+	private HashMap<String, FullCardSet> cardSetCache = new HashMap<String, FullCardSet>();
 	
 	/**
 	 * Returns the specified card in the form of a {@code Card} object. If the specified number does
@@ -64,7 +59,7 @@ public final class MTGUniverse {
 	 * @throws JsonMappingException if the upstream JSON does not map to a local class.
 	 * @throws IOException if a low-level I/O problem (unexpected end-of-input, network error) occurs.
 	 */
-	public static Card getCard(String setCode, String number) throws JsonParseException, JsonMappingException, IOException {
+	public Card getCard(String setCode, String number) throws JsonParseException, JsonMappingException, IOException {
 		Card card = null;
 		FullCardSet fullCardSet = getFullCardSet(setCode);
 		
@@ -89,7 +84,7 @@ public final class MTGUniverse {
 	 * @throws JsonMappingException if the upstream JSON does not map to {@code FullCardSet}.
 	 * @throws IOException if a low-level I/O problem (unexpected end-of-input, network error) occurs.
 	 */
-	public static FullCardSet getFullCardSet(String setCode) throws JsonParseException, JsonMappingException, IOException {
+	public FullCardSet getFullCardSet(String setCode) throws JsonParseException, JsonMappingException, IOException {
 		FullCardSet requestedSet = null;
 		String validCode = validateSetCode(setCode);
 		if (validCode != null) {
@@ -109,7 +104,7 @@ public final class MTGUniverse {
 	/**
 	 * @return a list of all card sets in the form of {@code CardSet} objects.
 	 */
-	public static List<CardSetInformation> getCardSetList() {
+	public List<CardSetInformation> getCardSetList() {
 		// if the list isn't cached, fetch and cache it
 		if (cardSets == null) {
 			try {
@@ -142,7 +137,7 @@ public final class MTGUniverse {
 	 * @param setCode the set code to be validated.
 	 * @return the valid form of the set code if any, null otherwise.
 	 */
-	public static String validateSetCode(String setCode) {
+	public String validateSetCode(String setCode) {
 		for (CardSetInformation cardSet : getCardSetList()) {
 			if (cardSet.getCode().equalsIgnoreCase(setCode)) {
 				return cardSet.getCode();
@@ -161,7 +156,7 @@ public final class MTGUniverse {
 	 * @throws JsonMappingException if the upstream JSON does not map to {@code FullCardSet}.
 	 * @throws IOException if a low-level I/O problem (unexpected end-of-input, network error) occurs.
 	 */
-	private static FullCardSet parseFullSet(JsonNode jsonTree) throws JsonMappingException, IOException {
+	private FullCardSet parseFullSet(JsonNode jsonTree) throws JsonMappingException, IOException {
 		
 		FullCardSet fcs = new FullCardSet();
 		
@@ -221,6 +216,5 @@ public final class MTGUniverse {
 		fcs.block = jsonTree.hasNonNull("block") ? jsonTree.get("block").asText() : null;
 		
 		return fcs;
-		
 	}
 }
\ No newline at end of file