Cleanup, and throw better exceptions

4 files changed, 46 insertions, 64 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
index 68ca8385b8a..730e2393b7f 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
@@ -31,18 +31,18 @@ import java.util.function.BiConsumer;
  */
 public class FastHit extends Hit {
 
-    private static final byte [] emptyGID = new byte[GlobalId.LENGTH];
+    private static final byte[] emptyGID = new byte[GlobalId.LENGTH];
     /** The index of the content node this hit originated at */
-    private int distributionKey = 0;
+    private int distributionKey;
 
     /** The local identifier of the content store for this hit on the node it originated at */
     private int partId;
 
     /** The global id of this document in the backend node which produced it */
-    private byte [] globalId;
+    private byte[] globalId;
 
     private transient byte[] sortData = null;
-    // TODO I suspect this one can be dropped.
+    // TODO: I suspect this one can be dropped.
     private transient Sorting sortDataSorting = null;
 
     /**
diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java
index 6067f85df9b..546a5d724a8 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java
@@ -18,15 +18,14 @@ public class SortDataHitSorter {
             return;
         }
         var fallbackComparator = fallbackOrderer.getComparator();
-        Collections.sort(hits, getComparator(sorting, fallbackComparator));
+        hits.sort(getComparator(sorting, fallbackComparator));
     }
 
     public static boolean isSortable(Hit hit, Sorting sorting) {
         if (sorting == null) {
             return false;
         }
-        if (hit instanceof FastHit) {
-            var fhit = (FastHit) hit;
+        if (hit instanceof FastHit fhit) {
             return fhit.hasSortData(sorting);
         } else {
             return false;
@@ -42,20 +41,14 @@ public class SortDataHitSorter {
     }
 
     private static int compareTwo(Hit left, Hit right, Sorting sorting) {
-        if (left == null || right == null || !(left instanceof FastHit) || !(right instanceof FastHit)) {
-            return 0;
-        }
-        FastHit fl = (FastHit) left;
-        FastHit fr = (FastHit) right;
+        if (!(left instanceof FastHit fl) || !(right instanceof FastHit fr)) return 0;
         return FastHit.compareSortData(fl, fr, sorting);
     }
 
     private static int compareWithFallback(Hit left, Hit right, Sorting sorting, Comparator<Hit> fallback) {
-        if (left == null || right == null || !(left instanceof FastHit) || !(right instanceof FastHit)) {
+        if (!(left instanceof FastHit fl) || !(right instanceof FastHit fr)) {
             return fallback.compare(left, right);
         }
-        FastHit fl = (FastHit) left;
-        FastHit fr = (FastHit) right;
         if (fl.hasSortData(sorting) && fr.hasSortData(sorting)) {
             return FastHit.compareSortData(fl, fr, sorting);
         } else {
diff --git a/container-search/src/main/java/com/yahoo/search/query/Sorting.java b/container-search/src/main/java/com/yahoo/search/query/Sorting.java
index 6b07b5ef1d5..ffaadebc443 100644
--- a/container-search/src/main/java/com/yahoo/search/query/Sorting.java
+++ b/container-search/src/main/java/com/yahoo/search/query/Sorting.java
@@ -161,6 +161,7 @@ public class Sorting implements Cloneable {
      */
     public List<FieldOrder> fieldOrders() { return fieldOrders; }
 
+    @Override
     public Sorting clone() {
         return new Sorting(this.fieldOrders);
     }
@@ -173,9 +174,7 @@ public class Sorting implements Cloneable {
     @Override
     public boolean equals(Object o) {
         if (o == this) return true;
-        if( ! (o instanceof Sorting)) return false;
-
-        Sorting ss = (Sorting) o;
+        if( ! (o instanceof Sorting ss)) return false;
         return fieldOrders.equals(ss.fieldOrders);
     }
 
@@ -305,15 +304,14 @@ public class Sorting implements Cloneable {
         public UcaSorter(String fieldName) { super(fieldName); }
 
         static private int strength2Collator(Strength strength) {
-            switch (strength) {
-                case PRIMARY: return Collator.PRIMARY;
-                case SECONDARY: return Collator.SECONDARY;
-                case TERTIARY: return Collator.TERTIARY;
-                case QUATERNARY: return Collator.QUATERNARY;
-                case IDENTICAL: return Collator.IDENTICAL;
-                case UNDEFINED: return Collator.PRIMARY;
-            }
-            return Collator.PRIMARY;
+            return switch (strength) {
+                case PRIMARY -> Collator.PRIMARY;
+                case SECONDARY -> Collator.SECONDARY;
+                case TERTIARY -> Collator.TERTIARY;
+                case QUATERNARY -> Collator.QUATERNARY;
+                case IDENTICAL -> Collator.IDENTICAL;
+                case UNDEFINED -> Collator.PRIMARY;
+            };
         }
 
         public void setLocale(String locale, Strength strength) {
@@ -323,15 +321,15 @@ public class Sorting implements Cloneable {
             try {
                 uloc = new ULocale(locale);
             } catch (Throwable e) {
-                throw new RuntimeException("ULocale("+locale+") failed with exception " + e.toString());
+                throw new IllegalArgumentException("ULocale '" + locale + "' failed", e);
             }
             try {
                 collator = Collator.getInstance(uloc);
                 if (collator == null) {
-                    throw new RuntimeException("No collator available for: " + locale);
+                    throw new IllegalArgumentException("No collator available for locale '" + locale + "'");
                 }
             } catch (Throwable e) {
-                throw new RuntimeException("Collator.getInstance(ULocale("+locale+")) failed with exception " + e.toString());
+                throw new RuntimeException("Collator.getInstance(ULocale(" + locale + ")) failed", e);
             }
             collator.setStrength(strength2Collator(strength));
             // collator.setDecomposition(Collator.CANONICAL_DECOMPOSITION);
@@ -343,19 +341,22 @@ public class Sorting implements Cloneable {
         public String getDecomposition() { return (collator.getDecomposition() == Collator.CANONICAL_DECOMPOSITION) ? "CANONICAL_DECOMPOSITION" : "NO_DECOMPOSITION"; }
 
         @Override
-        public String toSerialForm() { return "uca(" + getName() + ',' + locale + ',' + ((strength != Strength.UNDEFINED) ? strength.toString() : "PRIMARY") + ')'; }
+        public String toSerialForm() {
+            return "uca(" + getName() + ',' + locale + ',' +
+                   ((strength != Strength.UNDEFINED) ? strength.toString() : "PRIMARY") + ')';
+        }
 
         @Override
         public int hashCode() { return 1 + 3*locale.hashCode() + 5*strength.hashCode() + 7*super.hashCode(); }
 
         @Override
         public boolean equals(Object other) {
-            if (!(other instanceof UcaSorter)) {
-                return false;
-            }
+            if (this == other) return true;
+            if (!(other instanceof UcaSorter)) return false;
             return super.equals(other) && locale.equals(((UcaSorter)other).locale) && (strength == ((UcaSorter)other).strength);
         }
 
+        @Override
         public UcaSorter clone() {
             UcaSorter clone = (UcaSorter)super.clone();
             if (locale != null) {
@@ -365,6 +366,7 @@ public class Sorting implements Cloneable {
         }
 
         @SuppressWarnings({ "rawtypes", "unchecked" })
+        @Override
         public int compare(Comparable a, Comparable b) {
             if ((a instanceof String) && (b instanceof String)) {
                 return collator.compare((String)a, (String) b);
diff --git a/container-search/src/main/java/com/yahoo/search/result/FieldComparator.java b/container-search/src/main/java/com/yahoo/search/result/FieldComparator.java
index 0259dd66dbe..7f392d43753 100644
--- a/container-search/src/main/java/com/yahoo/search/result/FieldComparator.java
+++ b/container-search/src/main/java/com/yahoo/search/result/FieldComparator.java
@@ -17,7 +17,7 @@ import java.util.Comparator;
 public class FieldComparator extends ChainableComparator {
 
     /** The definition of sorting order */
-    private Sorting sorting;
+    private final Sorting sorting;
 
     /** Creates a field comparator using a sort order and having no chained comparator */
     public FieldComparator(Sorting sorting) {
@@ -32,7 +32,7 @@ public class FieldComparator extends ChainableComparator {
 
     /** Creates a comparator given a sorting, or returns null if the given sorting is null */
     public static FieldComparator create(Sorting sorting) {
-        if (sorting==null) return null;
+        if (sorting == null) return null;
         return new FieldComparator(sorting);
     }
 
@@ -41,7 +41,7 @@ public class FieldComparator extends ChainableComparator {
      * stored in hit fields.0
      * <p>
      * When one of the hits has the requested property and the other
-     * has not, the the hit containing the property precedes the one
+     * has not, the hit containing the property precedes the one
      * that does not.
      * <p>
      * There is no locale based sorting here, as the backend does
@@ -78,19 +78,14 @@ public class FieldComparator extends ChainableComparator {
             }
             Inspector sub = top.field(key);
             if (sub.valid()) {
-                switch (sub.type()) {
-                case EMPTY:
-                    return null;
-                case BOOL:
-                    return (sub.asBool() ? Boolean.TRUE : Boolean.FALSE);
-                case LONG:
-                    return sub.asLong();
-                case DOUBLE:
-                    return sub.asDouble();
-                case STRING:
-                    return sub.asString();
-                }
-                return sub.toString();
+                return switch (sub.type()) {
+                    case EMPTY -> null;
+                    case BOOL -> (sub.asBool() ? Boolean.TRUE : Boolean.FALSE);
+                    case LONG -> sub.asLong();
+                    case DOUBLE -> sub.asDouble();
+                    case STRING -> sub.asString();
+                    default -> sub.toString();
+                };
             }
         }
         // fallback value
@@ -115,15 +110,14 @@ public class FieldComparator extends ChainableComparator {
 
     @SuppressWarnings("rawtypes")
     private int compareValues(Object first, Object second, Sorting.AttributeSorter s) {
-        if (first == null) {
-            if (second == null) return 0;
-            return -1;
-        } else if (second == null) {
+        if (first == null)
+            return second == null ? 0 : -1;
+        else if (second == null)
             return 1;
-        }
+
         if (first.getClass().isInstance(second) && first instanceof Comparable) {
             // We now know:
-            // second is of a type which is a subclass of first's type
+            // Second is of a type which is a subclass of first's type
             // They both implement Comparable
             return s.compare((Comparable)first, (Comparable)second);
         } else {
@@ -133,14 +127,7 @@ public class FieldComparator extends ChainableComparator {
 
     @Override
     public String toString() {
-        StringBuilder b = new StringBuilder();
-        b.append("FieldComparator:");
-        if (sorting == null) {
-            b.append(" null");
-        } else {
-            b.append(sorting.toString());
-        }
-        return b.toString();
+        return "FieldComparator:" + (sorting == null ? " null" : sorting.toString());
     }
 
 }