ExerciseChallenge
Challenge
Six comparators, four of them broken
25 minintermediate1–12 yrs
Edge cases. You have to reason, and two valid fixes differ.
What this teaches
- Subtraction overflows, and the result is a wrong order rather than an exception
- A comparator that returns 0 for "similar" is not a total order
- A comparator reading a mutable field can break mid-sort with no bug in the comparator itself
- Inconsistency with equals is invisible in a List and destructive in a TreeSet
- Two of the six are correct, and one of those is still the wrong tool
Starter
Starter.java
import java.util.*;
/**
* Challenge: six comparators from real code. Four are broken.
*
* For each one: name the defect, write the input that demonstrates it, and say
* whether it throws or fails silently. The silent ones are the ones that reach
* production, so rank them by that rather than by how wrong they look.
*/
public class Starter {
record Order(long id, int amountMinor, String customer) {}
record Version(int major, int minor) {}
/** Mutable on purpose — this is what the fifth comparator reads. */
static final class Job {
final String name;
volatile int priority;
Job(String name, int priority) { this.name = name; this.priority = priority; }
@Override public String toString() { return name + "(" + priority + ")"; }
}
// ── 1 ─────────────────────────────────────────────────────────────────
// Sorting orders by id. Ids come from a 64-bit database sequence.
static final Comparator<Order> ONE = (a, b) -> (int) (a.id() - b.id());
// ── 2 ─────────────────────────────────────────────────────────────────
// "Amounts within one rupee are the same for reporting purposes."
static final Comparator<Order> TWO =
(a, b) -> Math.abs(a.amountMinor() - b.amountMinor()) <= 100
? 0 : Integer.compare(a.amountMinor(), b.amountMinor());
// ── 3 ─────────────────────────────────────────────────────────────────
// Largest amount first.
static final Comparator<Order> THREE = (a, b) -> b.amountMinor() - a.amountMinor();
// ── 4 ─────────────────────────────────────────────────────────────────
// Used as `new TreeSet<>(FOUR)` to hold the versions a client supports.
static final Comparator<Version> FOUR = Comparator.comparingInt(Version::major);
// ── 5 ─────────────────────────────────────────────────────────────────
// A worker thread adjusts Job.priority while the queue is being sorted.
static final Comparator<Job> FIVE = Comparator.comparingInt(j -> j.priority);
// ── 6 ─────────────────────────────────────────────────────────────────
// Customer, then amount descending, then id as a tiebreak.
static final Comparator<Order> SIX =
Comparator.comparing(Order::customer)
.thenComparing(Order::amountMinor, Comparator.reverseOrder())
.thenComparingLong(Order::id);
public static void main(String[] args) throws Exception {
// TODO 1: ONE. Two bugs are stacked here, not one. Find both, and
// build a two-element array that comes out in the wrong order.
// Does it throw? Write down the answer before you run it.
// TODO 2: TWO. Show that it is not transitive with three concrete
// amounts. Then sort 20 orders with it, and 500. Note which sizes
// complain and which do not, and say what that means about relying
// on the exception to catch this class of bug.
// TODO 3: THREE. It looks like ONE with the arguments swapped, and
// it has a different range of safe inputs. Say exactly when it breaks
// and write the input. Then fix it without using subtraction.
// TODO 4: FOUR. It is a perfectly valid total order — antisymmetric,
// transitive, no overflow. Add Version(2,0) and Version(2,7) to a
// TreeSet using it and print the size. Explain the result, and say why
// a HashSet would behave differently.
// TODO 5: FIVE. The comparator is correct. Sort a large Job[] while
// another thread mutates priorities, and describe what can go wrong.
// Name the fix — there are two, and they are not equivalent.
// TODO 6: SIX. Nothing is wrong with it. Say why each of the three
// keys is safe, and what `SIX.reversed()` would do that a reader
// might not expect.
// TODO 7: write the checker. Given a list and a comparator, verify
// antisymmetry and transitivity by brute force over all triples, and
// report the offending elements. Run it against all six. Which
// broken ones does it catch, and which does it miss?
}
}Run it locally:
cd exercises/java/searching-and-sorting/arrays-sort/02-challenge
javac Starter.java -d /tmp/out && java -cp /tmp/out StarterHints
Hint 1
For each one, ask three questions in order: can it overflow, is it transitive, and can what it reads change while the sort is running?
Hint 2
One of them never throws and never sorts correctly. That is the dangerous one — write the input that exposes it rather than trusting a small test.
Hint 3
One is a perfectly valid total order that still loses data. Look at where it is used, not at the comparator.
Hint 4
Two are correct. Say why, do not just leave them alone — being able to defend a correct comparator is the actual skill.
Done when
- Each broken comparator has a named defect and an input that demonstrates it
- You distinguished the ones that throw from the ones that silently mis-sort
- The mutable-field one is identified, and you can say why the comparator is not at fault
- You can state which two are correct and why
← Back to Which sort does Arrays.sort use, and why does it depend on the type?