diff --git a/README.md b/README.md index 1c654ef..b655842 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,7 @@ article; each module's own README has that article's version table, quickstart, | [`sequenced`](sequenced/) | Sequenced Collections in Java 21+: getFirst, getLast and reversed() | | [`immutable`](immutable/) | Immutable Collections in Java: List.of vs unmodifiableList vs copyOf | | [`cow`](cow/) | CopyOnWriteArrayList in Java: When It Wins (JMH) and When It Hurts | +| [`sorting`](sorting/) | Java Comparator & Comparable: The Complete Guide | ## License diff --git a/pom.xml b/pom.xml index e03aab7..fae17cb 100644 --- a/pom.xml +++ b/pom.xml @@ -32,6 +32,7 @@ sequenced immutable cow + sorting diff --git a/sorting/README.md b/sorting/README.md new file mode 100644 index 0000000..7ccc3e9 --- /dev/null +++ b/sorting/README.md @@ -0,0 +1,60 @@ +# sorting + +Companion code for the ankurm.com post *"Java Comparator & Comparable: The Complete Guide."* +Module in `java-core-examples`, the Java-core series. + +## Versions this was built and tested against + +| Component | Version | Notes | +|---|---|---| +| JDK | 25.0.4.1+1 (Temurin, LTS) | `Comparator.comparing`/`thenComparing`/`nullsFirst`/`nullsLast` shipped in Java 8 (2014), unchanged since. Records (used here as sort keys) shipped in Java 16 (2021). | +| JUnit Jupiter | 5.11.0 | | +| Maven | 3.9.11 | | + +## Quickstart + +```bash +export JAVA_HOME=/path/to/jdk-17-or-newer +mvn compile +java -cp target/classes com.ankurm.sorting.IntegerOverflowBugDemo +``` + +`scripts/run-all.sh` regenerates every file in `output/`. `scripts/run.sh ` runs one +demo ad hoc. + +## What's in here + +| File | What it shows | +|---|---| +| `Employee.java` | A plain POJO that deliberately does NOT implement `Comparable` - it has no one obvious order, which is the case `Comparator` exists for. | +| `Product.java` | A POJO that DOES implement `Comparable` - price is its one obvious natural order - using `Double.compare`, never subtraction. | +| `EmployeeNameComparator.java` + `OldSchoolComparatorDemo.java` | Pre-Java-8 Comparator construction: a named class and an anonymous inner class, run side by side. | +| `ComparableNaturalOrderDemo.java` | `Collections.sort(products)` calling `Product.compareTo()` automatically. | +| `ComparatorFactoryMethodsDemo.java` | Lambdas, `Comparator.comparing`/`comparingInt`/`comparingDouble`, `thenComparing`, `reversed`, `nullsFirst`/`nullsLast`, chained together. | +| `TreeSetOrderingDemo.java` | The same element type sorted two ways in a `TreeSet` - natural order from the no-arg constructor, a reversed custom order from the Comparator constructor. | +| `StockPrice.java` + `RecordOrderingDemo.java` | A record implementing `Comparable`; its accessor is `price()` not `getPrice()`; and the record-specific trap where a `compareTo` that does not use every component stops being consistent with the record's generated `equals()` - demonstrated by a `TreeSet` silently dropping an element. | +| `Ticket.java` + `IntegerOverflowBugDemo.java` | **The main exhibit.** `(a, b) -> a.val - b.val` reproduced exactly, sorted against `Integer.compare`, with the actual wrong order it produces for `Integer.MAX_VALUE` / `Integer.MIN_VALUE` input, checked independently of either comparator. | +| `SortingTest.java` | Pins all of the above as assertions, 9/9 passing - including the overflow bug and the record/equals inconsistency as regression tests, not just printouts. | +| `output/01-07` | Captured runs of all six demos plus the test suite. | + +## Notes worth knowing before reading the post + +- **The subtraction comparator does not crash. It just lies.** `output/06` is the full transcript: `(a,b) -> a.val - b.val` sorts `[A(10), B(MAX), C(-20), D(MIN), E(0), F(5), G(-1)]` into + `[G(-1), E(0), F(5), A(10), B(MAX), D(MIN), C(-20)]` - `B(MAX)` sitting directly in front of + `D(MIN)`, and `C(-20)` landing dead last. `Integer.compare` sorts the exact same list correctly. + Both are real `List.sort()` calls against the same seven elements - see `output/06` and + `SortingTest#subtractionComparatorProducesWrongOrderOnOverflow`. +- **A record's generated `equals()` uses every component. Your `compareTo()` doesn't have to.** If + it doesn't, `compareTo(x) == 0` stops implying `equals(x)` is true, and anything keyed on + `compareTo` - most importantly `TreeSet`/`TreeMap` - starts silently treating distinct records as + duplicates. See `output/05`, section 4. +- **`Collections.sort` / `List.sort` never threw here**, even with `Integer.MAX_VALUE` and + `Integer.MIN_VALUE` in a seven-element list. Java's sort implementation only runs its + contract-violation detector once a run is long enough to need merging (`TimSort`'s `MIN_MERGE`, + 32 elements by default); below that it is a plain insertion sort that will happily produce a + wrong-but-silent answer. A small reproduction case like this one will not save you from the bug + in production - it will just fail to warn you in a unit test with a short list. + +## License + +MIT - see the [repo-wide LICENSE](../LICENSE). diff --git a/sorting/output/01-old-school-comparator.txt b/sorting/output/01-old-school-comparator.txt new file mode 100644 index 0000000..7641b23 --- /dev/null +++ b/sorting/output/01-old-school-comparator.txt @@ -0,0 +1,11 @@ +=== 1. Named class: new EmployeeNameComparator() === +Employee{id=102, name='Alex', age=45, salary=120000.0} +Employee{id=104, name='Alex', age=35, salary=110000.0} +Employee{id=103, name='Bob', age=29, salary=85000.0} +Employee{id=101, name='Zoe', age=29, salary=80000.0} + +=== 2. Anonymous inner class, sorting by age === +Employee{id=101, name='Zoe', age=29, salary=80000.0} +Employee{id=103, name='Bob', age=29, salary=85000.0} +Employee{id=104, name='Alex', age=35, salary=110000.0} +Employee{id=102, name='Alex', age=45, salary=120000.0} diff --git a/sorting/output/02-comparable-natural-order.txt b/sorting/output/02-comparable-natural-order.txt new file mode 100644 index 0000000..73b8cc0 --- /dev/null +++ b/sorting/output/02-comparable-natural-order.txt @@ -0,0 +1,2 @@ +Before sort: [Keyboard($49.99), Monitor($299.0), Mouse($29.5), Webcam($79.99)] +After Collections.sort(products): [Mouse($29.5), Keyboard($49.99), Webcam($79.99), Monitor($299.0)] diff --git a/sorting/output/03-comparator-factory-methods.txt b/sorting/output/03-comparator-factory-methods.txt new file mode 100644 index 0000000..bb4c92f --- /dev/null +++ b/sorting/output/03-comparator-factory-methods.txt @@ -0,0 +1,38 @@ +=== 1. Lambda (type inferred) === +Employee{id=101, name='Zoe', age=29, salary=80000.0} +Employee{id=103, name='Bob', age=29, salary=85000.0} +Employee{id=104, name='Alex', age=35, salary=110000.0} +Employee{id=102, name='Alex', age=45, salary=120000.0} + +=== 2. Comparator.comparing(Employee::getName) - method reference === +Employee{id=102, name='Alex', age=45, salary=120000.0} +Employee{id=104, name='Alex', age=35, salary=110000.0} +Employee{id=103, name='Bob', age=29, salary=85000.0} +Employee{id=101, name='Zoe', age=29, salary=80000.0} + +=== 3. comparingInt/comparingDouble - avoid boxing for primitives === +Employee{id=102, name='Alex', age=45, salary=120000.0} +Employee{id=104, name='Alex', age=35, salary=110000.0} +Employee{id=103, name='Bob', age=29, salary=85000.0} +Employee{id=101, name='Zoe', age=29, salary=80000.0} + +=== 4. thenComparing - break ties left to right === +(Two employees are both named "Alex" - watch thenComparing break that tie by age) +Employee{id=104, name='Alex', age=35, salary=110000.0} +Employee{id=102, name='Alex', age=45, salary=120000.0} +Employee{id=103, name='Bob', age=29, salary=85000.0} +Employee{id=101, name='Zoe', age=29, salary=80000.0} + +=== 5. nullsFirst - an Employee whose name is null, sorted to the front === +Employee{id=105, name='null', age=40, salary=90000.0} +Employee{id=102, name='Alex', age=45, salary=120000.0} +Employee{id=104, name='Alex', age=35, salary=110000.0} +Employee{id=103, name='Bob', age=29, salary=85000.0} +Employee{id=101, name='Zoe', age=29, salary=80000.0} + +=== 6. nullsLast + CASE_INSENSITIVE_ORDER, chained with thenComparing === +Employee{id=104, name='Alex', age=35, salary=110000.0} +Employee{id=102, name='Alex', age=45, salary=120000.0} +Employee{id=103, name='Bob', age=29, salary=85000.0} +Employee{id=101, name='Zoe', age=29, salary=80000.0} +Employee{id=106, name='null', age=22, salary=60000.0} diff --git a/sorting/output/04-treeset-ordering.txt b/sorting/output/04-treeset-ordering.txt new file mode 100644 index 0000000..c383324 --- /dev/null +++ b/sorting/output/04-treeset-ordering.txt @@ -0,0 +1,2 @@ +Natural (no-arg TreeSet ctor, uses Product.compareTo): [Mouse($29.5), Keyboard($49.99), Monitor($299.0)] +Custom (Comparator ctor, descending price) : [Monitor($299.0), Keyboard($49.99), Mouse($29.5)] diff --git a/sorting/output/05-record-ordering.txt b/sorting/output/05-record-ordering.txt new file mode 100644 index 0000000..8247787 --- /dev/null +++ b/sorting/output/05-record-ordering.txt @@ -0,0 +1,23 @@ +=== 1. Comparator.comparing(StockPrice::ticker) - record accessor, no "get" prefix === +StockPrice[ticker=AAPL, price=189.5, volume=42000000] +StockPrice[ticker=MSFT, price=410.2, volume=18000000] +StockPrice[ticker=NVDA, price=118.75, volume=210000000] + +=== 2. Natural order via StockPrice.compareTo (price only) === +StockPrice[ticker=NVDA, price=118.75, volume=210000000] +StockPrice[ticker=AAPL, price=189.5, volume=42000000] +StockPrice[ticker=MSFT, price=410.2, volume=18000000] + +=== 3. Two DIFFERENT records, same price: compareTo==0 but equals()==false === +StockPrice[ticker=AAPL, price=150.0, volume=1000000].compareTo(StockPrice[ticker=MSFT, price=150.0, volume=2000000]) = 0 +StockPrice[ticker=AAPL, price=150.0, volume=1000000].equals(StockPrice[ticker=MSFT, price=150.0, volume=2000000]) = false + +=== 4. That inconsistency reaches a TreeSet: natural order (price-only compareTo) === +treeSet.add(appleAt150) -> true (first element) +treeSet.add(msftAt150) -> false (TreeSet used compareTo==0 to call this a duplicate) +treeSet now contains 1 element(s): [StockPrice[ticker=AAPL, price=150.0, volume=1000000]] +-> MSFT silently never entered the set, even though msftAt150.equals(appleAt150) is false + +=== 5. Fix: a Comparator that is consistent with equals (ticker, then price, then volume) === +treeSet.add(msftAt150) -> true (now distinguishes the two records) +treeSet now contains 2 element(s): [StockPrice[ticker=AAPL, price=150.0, volume=1000000], StockPrice[ticker=MSFT, price=150.0, volume=2000000]] diff --git a/sorting/output/06-integer-overflow-bug.txt b/sorting/output/06-integer-overflow-bug.txt new file mode 100644 index 0000000..1774a54 --- /dev/null +++ b/sorting/output/06-integer-overflow-bug.txt @@ -0,0 +1,14 @@ +=== 1. The contract violation in isolation === +buggyCompare(BIG=2147483647, NEG=-2) = -2147483647 (claims BIG < NEG: true) +correctCompare(BIG=2147483647, NEG=-2) = 1 (correctly says BIG > NEG: true) +BIG.val - NEG.val overflows int range because 2147483647 - (-2) = 2147483647 + 2 > Integer.MAX_VALUE, and int arithmetic wraps silently. + +=== 2. Sorting a real list with each comparator === +Input (unsorted): [A(10), B(2147483647), C(-20), D(-2147483648), E(0), F(5), G(-1)] +Sorted with (a,b) -> a.val - b.val : [G(-1), E(0), F(5), A(10), B(2147483647), D(-2147483648), C(-20)] +Sorted with Integer.compare(a.val, b.val) : [D(-2147483648), C(-20), G(-1), E(0), F(5), A(10), B(2147483647)] + +=== 3. Checking each result WITHOUT trusting either comparator === +buggyResult is actually ascending by .val? false +correctResult is actually ascending by .val? true +First inversion: B(2147483647) appears immediately before D(-2147483648), but 2147483647 > -2147483648 diff --git a/sorting/output/07-tests.txt b/sorting/output/07-tests.txt new file mode 100644 index 0000000..37611b2 --- /dev/null +++ b/sorting/output/07-tests.txt @@ -0,0 +1,4 @@ +------------------------------------------------------------------------------- +Test set: com.ankurm.sorting.SortingTest +------------------------------------------------------------------------------- +Tests run: 9, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.122 s -- in com.ankurm.sorting.SortingTest diff --git a/sorting/pom.xml b/sorting/pom.xml new file mode 100644 index 0000000..ca165e4 --- /dev/null +++ b/sorting/pom.xml @@ -0,0 +1,43 @@ + + + 4.0.0 + + + com.ankurm + java-core-examples + 1.0 + + + sorting + sorting + Comparable vs Comparator from first principles through the modern chaining API, TreeSet ordering, records as sort keys, and a real reproduction of the int-subtraction-overflow comparator bug. + + + + org.junit.jupiter + junit-jupiter + 5.11.0 + test + + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.13.0 + + 25 + + + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + + + + diff --git a/sorting/scripts/run-all.sh b/sorting/scripts/run-all.sh new file mode 100755 index 0000000..56758d7 --- /dev/null +++ b/sorting/scripts/run-all.sh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +# Regenerates every file in output/ from scratch: compile, run every demo, run the test suite. +set -euo pipefail +cd "$(dirname "$0")/.." + +mvn -q -B compile +CP="target/classes" + +java -cp "$CP" com.ankurm.sorting.OldSchoolComparatorDemo 2>&1 \ + | grep -vE 'JAVA_TOOL_OPTIONS|^WARNING' > output/01-old-school-comparator.txt +java -cp "$CP" com.ankurm.sorting.ComparableNaturalOrderDemo 2>&1 \ + | grep -vE 'JAVA_TOOL_OPTIONS|^WARNING' > output/02-comparable-natural-order.txt +java -cp "$CP" com.ankurm.sorting.ComparatorFactoryMethodsDemo 2>&1 \ + | grep -vE 'JAVA_TOOL_OPTIONS|^WARNING' > output/03-comparator-factory-methods.txt +java -cp "$CP" com.ankurm.sorting.TreeSetOrderingDemo 2>&1 \ + | grep -vE 'JAVA_TOOL_OPTIONS|^WARNING' > output/04-treeset-ordering.txt +java -cp "$CP" com.ankurm.sorting.RecordOrderingDemo 2>&1 \ + | grep -vE 'JAVA_TOOL_OPTIONS|^WARNING' > output/05-record-ordering.txt +java -cp "$CP" com.ankurm.sorting.IntegerOverflowBugDemo 2>&1 \ + | grep -vE 'JAVA_TOOL_OPTIONS|^WARNING' > output/06-integer-overflow-bug.txt + +mvn -q -B test +cp target/surefire-reports/com.ankurm.sorting.SortingTest.txt output/07-tests.txt + +echo "Regenerated output/01-07." diff --git a/sorting/scripts/run.sh b/sorting/scripts/run.sh new file mode 100755 index 0000000..75d7b13 --- /dev/null +++ b/sorting/scripts/run.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash +# Run one of the demo classes ad hoc, e.g.: ./scripts/run.sh IntegerOverflowBugDemo +set -euo pipefail +cd "$(dirname "$0")/.." +mvn -q -B compile +java -cp target/classes "com.ankurm.sorting.$1" diff --git a/sorting/src/main/java/com/ankurm/sorting/ComparableNaturalOrderDemo.java b/sorting/src/main/java/com/ankurm/sorting/ComparableNaturalOrderDemo.java new file mode 100644 index 0000000..226a731 --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/ComparableNaturalOrderDemo.java @@ -0,0 +1,28 @@ +package com.ankurm.sorting; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** + * {@link Product#compareTo(Product)} is called automatically by {@link Collections#sort(List)} + * because {@code Product implements Comparable} - no Comparator argument needed. This is + * the "natural order" half of the guide. Source: + * https://ankurm.com/git.app/asmhatre/java-core-examples/src/branch/main/sorting/src/main/java/com/ankurm/sorting/ComparableNaturalOrderDemo.java + */ +public final class ComparableNaturalOrderDemo { + + private ComparableNaturalOrderDemo() {} + + public static void main(String[] args) { + List products = new ArrayList<>(); + products.add(new Product("Keyboard", 49.99)); + products.add(new Product("Monitor", 299.00)); + products.add(new Product("Mouse", 29.50)); + products.add(new Product("Webcam", 79.99)); + + System.out.println("Before sort: " + products); + Collections.sort(products); // uses Product.compareTo() automatically + System.out.println("After Collections.sort(products): " + products); + } +} diff --git a/sorting/src/main/java/com/ankurm/sorting/ComparatorFactoryMethodsDemo.java b/sorting/src/main/java/com/ankurm/sorting/ComparatorFactoryMethodsDemo.java new file mode 100644 index 0000000..20a5731 --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/ComparatorFactoryMethodsDemo.java @@ -0,0 +1,67 @@ +package com.ankurm.sorting; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; + +import static com.ankurm.sorting.OldSchoolComparatorDemo.sampleEmployees; + +/** + * The modern replacement for {@link OldSchoolComparatorDemo}: lambdas, method references, and the + * static/default factory methods Java 8 added to {@link Comparator} itself - + * {@code comparing}/{@code comparingInt}/{@code comparingLong}/{@code comparingDouble}, + * {@code thenComparing}, and {@code reversed}. Same {@link Employee} data as the old-school demo + * on purpose, so the two output files can be read side by side. Source: + * https://ankurm.com/git.app/asmhatre/java-core-examples/src/branch/main/sorting/src/main/java/com/ankurm/sorting/ComparatorFactoryMethodsDemo.java + */ +public final class ComparatorFactoryMethodsDemo { + + private ComparatorFactoryMethodsDemo() {} + + public static void main(String[] args) { + List employees = sampleEmployees(); + + System.out.println("=== 1. Lambda (type inferred) ==="); + List byAgeLambda = new ArrayList<>(employees); + byAgeLambda.sort((e1, e2) -> Integer.compare(e1.getAge(), e2.getAge())); + byAgeLambda.forEach(System.out::println); + + System.out.println(); + System.out.println("=== 2. Comparator.comparing(Employee::getName) - method reference ==="); + List byName = new ArrayList<>(employees); + byName.sort(Comparator.comparing(Employee::getName)); + byName.forEach(System.out::println); + + System.out.println(); + System.out.println("=== 3. comparingInt/comparingDouble - avoid boxing for primitives ==="); + List bySalaryDesc = new ArrayList<>(employees); + bySalaryDesc.sort(Comparator.comparingDouble(Employee::getSalary).reversed()); + bySalaryDesc.forEach(System.out::println); + + System.out.println(); + System.out.println("=== 4. thenComparing - break ties left to right ==="); + List byNameThenAge = new ArrayList<>(employees); + Comparator byNameThenByAge = Comparator.comparing(Employee::getName) + .thenComparingInt(Employee::getAge); + byNameThenAge.sort(byNameThenByAge); + System.out.println("(Two employees are both named \"Alex\" - watch thenComparing break that tie by age)"); + byNameThenAge.forEach(System.out::println); + + System.out.println(); + System.out.println("=== 5. nullsFirst - an Employee whose name is null, sorted to the front ==="); + List withNullName = new ArrayList<>(employees); + withNullName.add(new Employee(105, null, 40, 90000)); + withNullName.sort(Comparator.comparing(Employee::getName, Comparator.nullsFirst(String::compareTo))); + withNullName.forEach(System.out::println); + + System.out.println(); + System.out.println("=== 6. nullsLast + CASE_INSENSITIVE_ORDER, chained with thenComparing ==="); + List withNullCaseInsensitive = new ArrayList<>(employees); + withNullCaseInsensitive.add(new Employee(106, null, 22, 60000)); + Comparator nameInsensitiveNullsLastThenAge = + Comparator.comparing(Employee::getName, Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER)) + .thenComparingInt(Employee::getAge); + withNullCaseInsensitive.sort(nameInsensitiveNullsLastThenAge); + withNullCaseInsensitive.forEach(System.out::println); + } +} diff --git a/sorting/src/main/java/com/ankurm/sorting/Employee.java b/sorting/src/main/java/com/ankurm/sorting/Employee.java new file mode 100644 index 0000000..a8c6a31 --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/Employee.java @@ -0,0 +1,34 @@ +package com.ankurm.sorting; + +/** + * A plain, mutable POJO with no ordering opinion of its own - on purpose. {@code Employee} does + * NOT implement {@link Comparable}, because in real codebases a lot of classes genuinely have no + * single "natural" order (should employees sort by name? age? salary? all three are equally + * valid), and that is exactly the case {@link java.util.Comparator} exists for. See the + * "Comparable vs Comparator" section of + * https://ankurm.com/java-comparator-and-comparable-the-complete-guide/ for the full argument. + */ +public final class Employee { + + private final long id; + private final String name; + private final int age; + private final double salary; + + public Employee(long id, String name, int age, double salary) { + this.id = id; + this.name = name; + this.age = age; + this.salary = salary; + } + + public long getId() { return id; } + public String getName() { return name; } + public int getAge() { return age; } + public double getSalary() { return salary; } + + @Override + public String toString() { + return "Employee{id=" + id + ", name='" + name + "', age=" + age + ", salary=" + salary + "}"; + } +} diff --git a/sorting/src/main/java/com/ankurm/sorting/EmployeeNameComparator.java b/sorting/src/main/java/com/ankurm/sorting/EmployeeNameComparator.java new file mode 100644 index 0000000..daa53a3 --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/EmployeeNameComparator.java @@ -0,0 +1,17 @@ +package com.ankurm.sorting; + +import java.util.Comparator; + +/** + * The "old school" way to express a sort strategy: a dedicated named class implementing + * {@link Comparator}. Reusable and self-documenting by its class name, but one source file per + * sort order gets old fast. {@link OldSchoolComparatorDemo} runs this alongside its Java 8+ + * replacement so the difference is something you can see rather than take on faith. See + * https://ankurm.com/java-comparator-and-comparable-the-complete-guide/. + */ +public final class EmployeeNameComparator implements Comparator { + @Override + public int compare(Employee e1, Employee e2) { + return e1.getName().compareTo(e2.getName()); + } +} diff --git a/sorting/src/main/java/com/ankurm/sorting/IntegerOverflowBugDemo.java b/sorting/src/main/java/com/ankurm/sorting/IntegerOverflowBugDemo.java new file mode 100644 index 0000000..7ad137f --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/IntegerOverflowBugDemo.java @@ -0,0 +1,93 @@ +package com.ankurm.sorting; + +import java.util.ArrayList; +import java.util.List; + +/** + * The single most repeated piece of wrong Comparator advice: {@code return a.val - b.val}. It + * "works" for small numbers, which is exactly why it survives code review for years before it + * meets a value near {@link Integer#MAX_VALUE} or {@link Integer#MIN_VALUE} and silently produces + * the wrong order - no exception, no crash, just a list that looks plausible and is not actually + * sorted. This class sorts the SAME list with the buggy comparator and with the correct one and + * prints both real results, plus an independent (comparator-free) check of whether each result is + * actually ascending. {@link SortingTest#subtractionComparatorProducesWrongOrderOnOverflow()} + * pins these exact numbers as a regression test. Source: + * https://ankurm.com/git.app/asmhatre/java-core-examples/src/branch/main/sorting/src/main/java/com/ankurm/sorting/IntegerOverflowBugDemo.java + */ +public final class IntegerOverflowBugDemo { + + private IntegerOverflowBugDemo() {} + + /** The bug, verbatim: subtraction as a comparator. */ + public static int buggyCompare(Ticket a, Ticket b) { + return a.val - b.val; + } + + /** The fix: never subtract, ask the JDK. */ + public static int correctCompare(Ticket a, Ticket b) { + return Integer.compare(a.val, b.val); + } + + public static void main(String[] args) { + System.out.println("=== 1. The contract violation in isolation ==="); + Ticket big = new Ticket("BIG", Integer.MAX_VALUE); + Ticket negative = new Ticket("NEG", -2); + int buggySign = buggyCompare(big, negative); + int correctSign = correctCompare(big, negative); + System.out.println("buggyCompare(BIG=" + big.val + ", NEG=" + negative.val + ") = " + buggySign + + " (claims BIG < NEG: " + (buggySign < 0) + ")"); + System.out.println("correctCompare(BIG=" + big.val + ", NEG=" + negative.val + ") = " + correctSign + + " (correctly says BIG > NEG: " + (correctSign > 0) + ")"); + System.out.println("BIG.val - NEG.val overflows int range because " + big.val + " - (" + negative.val + + ") = " + big.val + " + " + (-negative.val) + " > Integer.MAX_VALUE, and int arithmetic wraps silently."); + + System.out.println(); + System.out.println("=== 2. Sorting a real list with each comparator ==="); + List tickets = List.of( + new Ticket("A", 10), + new Ticket("B", Integer.MAX_VALUE), + new Ticket("C", -20), + new Ticket("D", Integer.MIN_VALUE), + new Ticket("E", 0), + new Ticket("F", 5), + new Ticket("G", -1) + ); + System.out.println("Input (unsorted): " + tickets); + + List buggyResult = new ArrayList<>(tickets); + buggyResult.sort(IntegerOverflowBugDemo::buggyCompare); + System.out.println("Sorted with (a,b) -> a.val - b.val : " + buggyResult); + + List correctResult = new ArrayList<>(tickets); + correctResult.sort(IntegerOverflowBugDemo::correctCompare); + System.out.println("Sorted with Integer.compare(a.val, b.val) : " + correctResult); + + System.out.println(); + System.out.println("=== 3. Checking each result WITHOUT trusting either comparator ==="); + System.out.println("buggyResult is actually ascending by .val? " + isAscendingByVal(buggyResult)); + System.out.println("correctResult is actually ascending by .val? " + isAscendingByVal(correctResult)); + if (!isAscendingByVal(buggyResult)) { + reportFirstInversion(buggyResult); + } + } + + /** Ground truth, using plain {@code <=} on the actual int values - no comparator involved. */ + static boolean isAscendingByVal(List tickets) { + for (int i = 1; i < tickets.size(); i++) { + if (tickets.get(i - 1).val > tickets.get(i).val) { + return false; + } + } + return true; + } + + private static void reportFirstInversion(List tickets) { + for (int i = 1; i < tickets.size(); i++) { + if (tickets.get(i - 1).val > tickets.get(i).val) { + System.out.println("First inversion: " + tickets.get(i - 1) + " appears immediately before " + + tickets.get(i) + ", but " + tickets.get(i - 1).val + " > " + tickets.get(i).val); + return; + } + } + } +} diff --git a/sorting/src/main/java/com/ankurm/sorting/OldSchoolComparatorDemo.java b/sorting/src/main/java/com/ankurm/sorting/OldSchoolComparatorDemo.java new file mode 100644 index 0000000..0d9675f --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/OldSchoolComparatorDemo.java @@ -0,0 +1,47 @@ +package com.ankurm.sorting; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +/** + * Pre-Java-8 Comparator construction: a named class ({@link EmployeeNameComparator}) and an + * anonymous inner class. Both still compile and run today - nothing here is deprecated - they are + * just the verbose ancestors of {@link ComparatorFactoryMethodsDemo}'s one-liners. Run this first + * if you want to feel why Java 8 changed things. Source: + * https://ankurm.com/git.app/asmhatre/java-core-examples/src/branch/main/sorting/src/main/java/com/ankurm/sorting/OldSchoolComparatorDemo.java + */ +public final class OldSchoolComparatorDemo { + + private OldSchoolComparatorDemo() {} + + public static void main(String[] args) { + List employees = sampleEmployees(); + + System.out.println("=== 1. Named class: new EmployeeNameComparator() ==="); + List byName = new ArrayList<>(employees); + Collections.sort(byName, new EmployeeNameComparator()); + byName.forEach(System.out::println); + + System.out.println(); + System.out.println("=== 2. Anonymous inner class, sorting by age ==="); + List byAge = new ArrayList<>(employees); + Collections.sort(byAge, new Comparator() { + @Override + public int compare(Employee e1, Employee e2) { + return Integer.compare(e1.getAge(), e2.getAge()); + } + }); + byAge.forEach(System.out::println); + } + + static List sampleEmployees() { + List list = new ArrayList<>(); + list.add(new Employee(101, "Zoe", 29, 80000)); + list.add(new Employee(102, "Alex", 45, 120000)); + list.add(new Employee(103, "Bob", 29, 85000)); + list.add(new Employee(104, "Alex", 35, 110000)); + return list; + } +} diff --git a/sorting/src/main/java/com/ankurm/sorting/Product.java b/sorting/src/main/java/com/ankurm/sorting/Product.java new file mode 100644 index 0000000..d9807d8 --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/Product.java @@ -0,0 +1,36 @@ +package com.ankurm.sorting; + +/** + * Unlike {@link Employee}, a {@code Product} genuinely has one obvious default order: ascending + * price. That makes it the right candidate for {@link Comparable} - the ordering is intrinsic to + * what the object means, not a presentation choice made at one call site. See + * {@link #compareTo(Product)} and the "Comparable - natural ordering" section of + * https://ankurm.com/java-comparator-and-comparable-the-complete-guide/. + */ +public final class Product implements Comparable { + + private final String name; + private final double price; + + public Product(String name, double price) { + this.name = name; + this.price = price; + } + + public String getName() { return name; } + public double getPrice() { return price; } + + @Override + public int compareTo(Product other) { + // Double.compare, not (this.price - other.price): subtraction on floating-point values + // loses precision and does not even reliably produce the right SIGN. See + // IntegerOverflowBugDemo for the int-subtraction analogue of this mistake, reproduced + // with a real failing comparison rather than just asserted in prose. + return Double.compare(this.price, other.price); + } + + @Override + public String toString() { + return name + "($" + price + ")"; + } +} diff --git a/sorting/src/main/java/com/ankurm/sorting/RecordOrderingDemo.java b/sorting/src/main/java/com/ankurm/sorting/RecordOrderingDemo.java new file mode 100644 index 0000000..613721f --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/RecordOrderingDemo.java @@ -0,0 +1,69 @@ +package com.ankurm.sorting; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.TreeSet; + +/** + * Records as sort keys. Part 1 sorts a list of {@link StockPrice} records with + * {@code Comparator.comparing} using accessor-method references ({@code StockPrice::ticker}, not + * {@code getTicker}). Part 2 is the one most people do not expect: two different + * {@code StockPrice} records that happen to share a price compare as equal + * ({@code compareTo == 0}) but are NOT {@code equals()} - and a {@link TreeSet}, which uses + * {@code compareTo} rather than {@code equals} to detect duplicates, silently keeps only one of + * them. Source: + * https://ankurm.com/git.app/asmhatre/java-core-examples/src/branch/main/sorting/src/main/java/com/ankurm/sorting/RecordOrderingDemo.java + */ +public final class RecordOrderingDemo { + + private RecordOrderingDemo() {} + + public static void main(String[] args) { + List quotes = new ArrayList<>(List.of( + new StockPrice("MSFT", 410.20, 18_000_000), + new StockPrice("AAPL", 189.50, 42_000_000), + new StockPrice("NVDA", 118.75, 210_000_000) + )); + + System.out.println("=== 1. Comparator.comparing(StockPrice::ticker) - record accessor, no \"get\" prefix ==="); + List byTicker = new ArrayList<>(quotes); + byTicker.sort(Comparator.comparing(StockPrice::ticker)); + byTicker.forEach(System.out::println); + + System.out.println(); + System.out.println("=== 2. Natural order via StockPrice.compareTo (price only) ==="); + List byPrice = new ArrayList<>(quotes); + byPrice.sort(Comparator.naturalOrder()); + byPrice.forEach(System.out::println); + + System.out.println(); + System.out.println("=== 3. Two DIFFERENT records, same price: compareTo==0 but equals()==false ==="); + StockPrice appleAt150 = new StockPrice("AAPL", 150.00, 1_000_000); + StockPrice msftAt150 = new StockPrice("MSFT", 150.00, 2_000_000); + System.out.println(appleAt150 + ".compareTo(" + msftAt150 + ") = " + appleAt150.compareTo(msftAt150)); + System.out.println(appleAt150 + ".equals(" + msftAt150 + ") = " + appleAt150.equals(msftAt150)); + + System.out.println(); + System.out.println("=== 4. That inconsistency reaches a TreeSet: natural order (price-only compareTo) ==="); + TreeSet treeSetNaturalOrder = new TreeSet<>(); + treeSetNaturalOrder.add(appleAt150); + boolean secondAdded = treeSetNaturalOrder.add(msftAt150); + System.out.println("treeSet.add(appleAt150) -> true (first element)"); + System.out.println("treeSet.add(msftAt150) -> " + secondAdded + " (TreeSet used compareTo==0 to call this a duplicate)"); + System.out.println("treeSet now contains " + treeSetNaturalOrder.size() + " element(s): " + treeSetNaturalOrder); + System.out.println("-> MSFT silently never entered the set, even though msftAt150.equals(appleAt150) is false"); + + System.out.println(); + System.out.println("=== 5. Fix: a Comparator that is consistent with equals (ticker, then price, then volume) ==="); + Comparator consistentWithEquals = + Comparator.comparing(StockPrice::ticker) + .thenComparingDouble(StockPrice::price) + .thenComparingInt(StockPrice::volume); + TreeSet treeSetConsistent = new TreeSet<>(consistentWithEquals); + treeSetConsistent.add(appleAt150); + boolean secondAddedFixed = treeSetConsistent.add(msftAt150); + System.out.println("treeSet.add(msftAt150) -> " + secondAddedFixed + " (now distinguishes the two records)"); + System.out.println("treeSet now contains " + treeSetConsistent.size() + " element(s): " + treeSetConsistent); + } +} diff --git a/sorting/src/main/java/com/ankurm/sorting/StockPrice.java b/sorting/src/main/java/com/ankurm/sorting/StockPrice.java new file mode 100644 index 0000000..e8fe784 --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/StockPrice.java @@ -0,0 +1,27 @@ +package com.ankurm.sorting; + +/** + * A record implementing {@link Comparable}. Two things are different from a plain POJO like + * {@link Product}: the accessor Java generates for {@code price} is called {@code price()}, not + * {@code getPrice()} - so a method reference used with {@code Comparator.comparing} looks like + * {@code StockPrice::price}, not {@code StockPrice::getPrice} - and {@code equals()}/ + * {@code hashCode()} are generated from every component ({@code ticker}, {@code price} + * AND {@code volume}). {@link #compareTo(StockPrice)} here only looks at {@code price}, which is + * a completely ordinary thing to want - but it means {@code compareTo(x) == 0} no longer implies + * {@code equals(x)}. {@link RecordOrderingDemo} shows exactly what that costs in a + * {@link java.util.TreeSet}. Source: + * https://ankurm.com/git.app/asmhatre/java-core-examples/src/branch/main/sorting/src/main/java/com/ankurm/sorting/StockPrice.java + */ +public record StockPrice(String ticker, double price, int volume) implements Comparable { + + public StockPrice { + if (price < 0) { + throw new IllegalArgumentException("price cannot be negative: " + price); + } + } + + @Override + public int compareTo(StockPrice other) { + return Double.compare(this.price, other.price); + } +} diff --git a/sorting/src/main/java/com/ankurm/sorting/Ticket.java b/sorting/src/main/java/com/ankurm/sorting/Ticket.java new file mode 100644 index 0000000..616c6bc --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/Ticket.java @@ -0,0 +1,21 @@ +package com.ankurm.sorting; + +/** + * A minimal holder for {@link IntegerOverflowBugDemo} - just an int field, public and final, so + * the buggy comparator can be written exactly as it appears in real code review threads: + * {@code (a, b) -> a.val - b.val}. Nothing else about this class matters for the demo. + */ +public final class Ticket { + public final String label; + public final int val; + + public Ticket(String label, int val) { + this.label = label; + this.val = val; + } + + @Override + public String toString() { + return label + "(" + val + ")"; + } +} diff --git a/sorting/src/main/java/com/ankurm/sorting/TreeSetOrderingDemo.java b/sorting/src/main/java/com/ankurm/sorting/TreeSetOrderingDemo.java new file mode 100644 index 0000000..e94ac91 --- /dev/null +++ b/sorting/src/main/java/com/ankurm/sorting/TreeSetOrderingDemo.java @@ -0,0 +1,33 @@ +package com.ankurm.sorting; + +import java.util.Comparator; +import java.util.TreeSet; + +/** + * {@link TreeSet} is the clearest place to see Comparable and Comparator meet: the no-arg + * constructor uses {@link Product#compareTo(Product)} (natural order), while the + * Comparator-accepting constructor overrides it - same element type, two different orderings, + * neither one touching {@code Product}'s source. Source: + * https://ankurm.com/git.app/asmhatre/java-core-examples/src/branch/main/sorting/src/main/java/com/ankurm/sorting/TreeSetOrderingDemo.java + */ +public final class TreeSetOrderingDemo { + + private TreeSetOrderingDemo() {} + + public static void main(String[] args) { + // Uses Product.compareTo() -- natural order (ascending price) + TreeSet byNaturalOrder = new TreeSet<>(); + byNaturalOrder.add(new Product("Monitor", 299.0)); + byNaturalOrder.add(new Product("Mouse", 29.5)); + byNaturalOrder.add(new Product("Keyboard", 49.99)); + System.out.println("Natural (no-arg TreeSet ctor, uses Product.compareTo): " + byNaturalOrder); + + // Pass a Comparator to override order (descending price) - Product's source is untouched + TreeSet byPriceDesc = + new TreeSet<>(Comparator.comparingDouble(Product::getPrice).reversed()); + byPriceDesc.add(new Product("Monitor", 299.0)); + byPriceDesc.add(new Product("Mouse", 29.5)); + byPriceDesc.add(new Product("Keyboard", 49.99)); + System.out.println("Custom (Comparator ctor, descending price) : " + byPriceDesc); + } +} diff --git a/sorting/src/test/java/com/ankurm/sorting/SortingTest.java b/sorting/src/test/java/com/ankurm/sorting/SortingTest.java new file mode 100644 index 0000000..4dc2536 --- /dev/null +++ b/sorting/src/test/java/com/ankurm/sorting/SortingTest.java @@ -0,0 +1,181 @@ +package com.ankurm.sorting; + +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.TreeSet; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Pins the contract, not just the happy path, for every demo in this module. The most important + * test here is {@link #subtractionComparatorProducesWrongOrderOnOverflow()} - it is the assertion + * version of {@link IntegerOverflowBugDemo}, and it is what makes "the subtraction trick can + * overflow" a checked fact in this repository rather than a warning in a blog post. + */ +class SortingTest { + + @Test + void oldSchoolNamedClassAndModernComparingAgreeOnOrder() { + List employees = OldSchoolComparatorDemo.sampleEmployees(); + + List viaNamedClass = new ArrayList<>(employees); + viaNamedClass.sort(new EmployeeNameComparator()); + + List viaComparing = new ArrayList<>(employees); + viaComparing.sort(Comparator.comparing(Employee::getName)); + + assertEquals( + viaNamedClass.stream().map(Employee::getName).toList(), + viaComparing.stream().map(Employee::getName).toList(), + "a hand-written Comparator class and Comparator.comparing() must produce the same order" + ); + } + + @Test + void thenComparingBreaksTiesLeftToRight() { + // sample data has two employees named "Alex": age 45 and age 35 + List employees = new ArrayList<>(OldSchoolComparatorDemo.sampleEmployees()); + employees.sort(Comparator.comparing(Employee::getName).thenComparingInt(Employee::getAge)); + + List alexes = employees.stream().filter(e -> "Alex".equals(e.getName())).toList(); + assertEquals(2, alexes.size(), "sample data must contain exactly two employees named Alex"); + assertEquals(35, alexes.get(0).getAge(), "thenComparing must place the younger Alex (35) first"); + assertEquals(45, alexes.get(1).getAge(), "thenComparing must place the older Alex (45) second"); + } + + @Test + void nullsFirstPutsTheNullNameAtTheFront() { + List employees = new ArrayList<>(OldSchoolComparatorDemo.sampleEmployees()); + employees.add(new Employee(105, null, 40, 90000)); + employees.sort(Comparator.comparing(Employee::getName, Comparator.nullsFirst(String::compareTo))); + + assertEquals(null, employees.get(0).getName(), "nullsFirst must sort the null-named employee to index 0"); + } + + @Test + void comparableNaturalOrderSortsProductsAscendingByPrice() { + List products = new ArrayList<>(List.of( + new Product("Keyboard", 49.99), + new Product("Monitor", 299.00), + new Product("Mouse", 29.50), + new Product("Webcam", 79.99) + )); + products.sort(null); // null Comparator to List.sort means "use natural order", i.e. compareTo + + assertEquals(List.of("Mouse", "Keyboard", "Webcam", "Monitor"), + products.stream().map(Product::getName).toList()); + } + + @Test + void treeSetNaturalOrderIsAscendingAndCustomComparatorReversesIt() { + TreeSet natural = new TreeSet<>(); + natural.add(new Product("Monitor", 299.0)); + natural.add(new Product("Mouse", 29.5)); + natural.add(new Product("Keyboard", 49.99)); + assertEquals(List.of("Mouse", "Keyboard", "Monitor"), + natural.stream().map(Product::getName).toList()); + + TreeSet descending = new TreeSet<>(Comparator.comparingDouble(Product::getPrice).reversed()); + descending.add(new Product("Monitor", 299.0)); + descending.add(new Product("Mouse", 29.5)); + descending.add(new Product("Keyboard", 49.99)); + assertEquals(List.of("Monitor", "Keyboard", "Mouse"), + descending.stream().map(Product::getName).toList()); + } + + @Test + void recordAccessorMethodReferenceSortsByTicker() { + List quotes = new ArrayList<>(List.of( + new StockPrice("MSFT", 410.20, 18_000_000), + new StockPrice("AAPL", 189.50, 42_000_000), + new StockPrice("NVDA", 118.75, 210_000_000) + )); + quotes.sort(Comparator.comparing(StockPrice::ticker)); + assertEquals(List.of("AAPL", "MSFT", "NVDA"), quotes.stream().map(StockPrice::ticker).toList()); + } + + @Test + void recordCompactConstructorRejectsNegativePrice() { + assertThrows(IllegalArgumentException.class, () -> new StockPrice("BAD", -1.0, 100)); + } + + @Test + void priceOnlyCompareToIsInconsistentWithRecordEqualsAndDropsAnElementFromATreeSet() { + StockPrice appleAt150 = new StockPrice("AAPL", 150.00, 1_000_000); + StockPrice msftAt150 = new StockPrice("MSFT", 150.00, 2_000_000); + + // The core inconsistency: compareTo says "equal", equals() says "different". + assertEquals(0, appleAt150.compareTo(msftAt150), "same price must compare as 0 under the price-only compareTo"); + assertFalse(appleAt150.equals(msftAt150), "different ticker/volume must make the records unequal"); + + // And here is what that inconsistency actually costs: TreeSet uses compareTo, so it + // thinks the second add is a duplicate and silently drops it. + TreeSet treeSet = new TreeSet<>(); + assertTrue(treeSet.add(appleAt150)); + assertFalse(treeSet.add(msftAt150), "TreeSet must treat compareTo==0 as a duplicate and refuse the add"); + assertEquals(1, treeSet.size(), "the record with equal price was silently dropped, even though it is not equals()"); + + // Fixed version: a Comparator consistent with equals() keeps both. + Comparator consistentWithEquals = + Comparator.comparing(StockPrice::ticker) + .thenComparingDouble(StockPrice::price) + .thenComparingInt(StockPrice::volume); + TreeSet fixedTreeSet = new TreeSet<>(consistentWithEquals); + assertTrue(fixedTreeSet.add(appleAt150)); + assertTrue(fixedTreeSet.add(msftAt150), "a Comparator consistent with equals() must keep both records"); + assertEquals(2, fixedTreeSet.size()); + } + + @Test + void subtractionComparatorProducesWrongOrderOnOverflow() { + Ticket big = new Ticket("BIG", Integer.MAX_VALUE); + Ticket negative = new Ticket("NEG", -2); + + // The contract violation in isolation: BIG.val (2147483647) is greater than NEG.val (-2), + // so a correct comparator must return a POSITIVE number for compare(big, negative). + int buggySign = IntegerOverflowBugDemo.buggyCompare(big, negative); + int correctSign = IntegerOverflowBugDemo.correctCompare(big, negative); + assertTrue(buggySign < 0, + "the subtraction comparator must overflow and wrongly report BIG < NEG (got " + buggySign + ")"); + assertTrue(correctSign > 0, + "Integer.compare must correctly report BIG > NEG (got " + correctSign + ")"); + + // The real sort: same input list, two comparators, two DIFFERENT and NOT BOTH correct results. + List input = List.of( + new Ticket("A", 10), new Ticket("B", Integer.MAX_VALUE), new Ticket("C", -20), + new Ticket("D", Integer.MIN_VALUE), new Ticket("E", 0), new Ticket("F", 5), new Ticket("G", -1) + ); + + List buggyResult = new ArrayList<>(input); + buggyResult.sort(IntegerOverflowBugDemo::buggyCompare); + + List correctResult = new ArrayList<>(input); + correctResult.sort(IntegerOverflowBugDemo::correctCompare); + + // Ground truth, independent of either comparator. + assertTrue(IntegerOverflowBugDemo.isAscendingByVal(correctResult), + "Integer.compare's result must actually be ascending by .val"); + assertFalse(IntegerOverflowBugDemo.isAscendingByVal(buggyResult), + "the subtraction comparator's result must NOT actually be ascending by .val - this is the bug"); + + assertNotEquals( + correctResult.stream().map(t -> t.val).toList(), + buggyResult.stream().map(t -> t.val).toList(), + "the two comparators must disagree on the resulting order for this input" + ); + + // Integer.MIN_VALUE is the smallest possible value in the list; a correct ascending sort + // must put it first. The buggy comparator must NOT, because MIN_VALUE - anything-positive + // also overflows (wraps to a large positive number, making MIN_VALUE look "greater"). + assertEquals(Integer.MIN_VALUE, correctResult.get(0).val); + assertNotEquals(Integer.MIN_VALUE, buggyResult.get(0).val, + "the buggy comparator must fail to place Integer.MIN_VALUE first"); + } +}