sorting: Comparable vs Comparator from first principles, comparing/thenComparing/nullsFirst chaining, records as sort keys (and the record/equals compareTo trap), and a real reproduction of the int-subtraction-overflow comparator bug
Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01YXCrLgRKFgCh9RHKW8xaqJ
This commit is contained in:
@@ -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 <ClassName>` 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).
|
||||
@@ -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}
|
||||
@@ -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)]
|
||||
@@ -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}
|
||||
@@ -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)]
|
||||
@@ -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]]
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -0,0 +1,43 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<project xmlns="http://maven.apache.org/POM/4.0.0"
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
|
||||
<modelVersion>4.0.0</modelVersion>
|
||||
|
||||
<parent>
|
||||
<groupId>com.ankurm</groupId>
|
||||
<artifactId>java-core-examples</artifactId>
|
||||
<version>1.0</version>
|
||||
</parent>
|
||||
|
||||
<artifactId>sorting</artifactId>
|
||||
<name>sorting</name>
|
||||
<description>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.</description>
|
||||
|
||||
<dependencies>
|
||||
<dependency>
|
||||
<groupId>org.junit.jupiter</groupId>
|
||||
<artifactId>junit-jupiter</artifactId>
|
||||
<version>5.11.0</version>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
</dependencies>
|
||||
|
||||
<build>
|
||||
<plugins>
|
||||
<plugin>
|
||||
<groupId>org.apache.maven.plugins</groupId>
|
||||
<artifactId>maven-compiler-plugin</artifactId>
|
||||
<version>3.13.0</version>
|
||||
<configuration>
|
||||
<release>25</release>
|
||||
</configuration>
|
||||
</plugin>
|
||||
<plugin>
|
||||
<groupId>org.apache.maven.plugins</groupId>
|
||||
<artifactId>maven-surefire-plugin</artifactId>
|
||||
<version>3.2.5</version>
|
||||
</plugin>
|
||||
</plugins>
|
||||
</build>
|
||||
</project>
|
||||
Executable
+25
@@ -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."
|
||||
Executable
+6
@@ -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"
|
||||
@@ -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<Product>} - 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<Product> 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);
|
||||
}
|
||||
}
|
||||
@@ -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<Employee> employees = sampleEmployees();
|
||||
|
||||
System.out.println("=== 1. Lambda (type inferred) ===");
|
||||
List<Employee> 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<Employee> 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<Employee> 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<Employee> byNameThenAge = new ArrayList<>(employees);
|
||||
Comparator<Employee> 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<Employee> 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<Employee> withNullCaseInsensitive = new ArrayList<>(employees);
|
||||
withNullCaseInsensitive.add(new Employee(106, null, 22, 60000));
|
||||
Comparator<Employee> nameInsensitiveNullsLastThenAge =
|
||||
Comparator.comparing(Employee::getName, Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER))
|
||||
.thenComparingInt(Employee::getAge);
|
||||
withNullCaseInsensitive.sort(nameInsensitiveNullsLastThenAge);
|
||||
withNullCaseInsensitive.forEach(System.out::println);
|
||||
}
|
||||
}
|
||||
@@ -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 + "}";
|
||||
}
|
||||
}
|
||||
@@ -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<Employee> {
|
||||
@Override
|
||||
public int compare(Employee e1, Employee e2) {
|
||||
return e1.getName().compareTo(e2.getName());
|
||||
}
|
||||
}
|
||||
@@ -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<Ticket> 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<Ticket> buggyResult = new ArrayList<>(tickets);
|
||||
buggyResult.sort(IntegerOverflowBugDemo::buggyCompare);
|
||||
System.out.println("Sorted with (a,b) -> a.val - b.val : " + buggyResult);
|
||||
|
||||
List<Ticket> 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<Ticket> 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<Ticket> 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;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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<Employee> employees = sampleEmployees();
|
||||
|
||||
System.out.println("=== 1. Named class: new EmployeeNameComparator() ===");
|
||||
List<Employee> 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<Employee> byAge = new ArrayList<>(employees);
|
||||
Collections.sort(byAge, new Comparator<Employee>() {
|
||||
@Override
|
||||
public int compare(Employee e1, Employee e2) {
|
||||
return Integer.compare(e1.getAge(), e2.getAge());
|
||||
}
|
||||
});
|
||||
byAge.forEach(System.out::println);
|
||||
}
|
||||
|
||||
static List<Employee> sampleEmployees() {
|
||||
List<Employee> 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;
|
||||
}
|
||||
}
|
||||
@@ -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<Product> {
|
||||
|
||||
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 + ")";
|
||||
}
|
||||
}
|
||||
@@ -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<StockPrice> 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<StockPrice> 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<StockPrice> 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<StockPrice> 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<StockPrice> consistentWithEquals =
|
||||
Comparator.comparing(StockPrice::ticker)
|
||||
.thenComparingDouble(StockPrice::price)
|
||||
.thenComparingInt(StockPrice::volume);
|
||||
TreeSet<StockPrice> 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);
|
||||
}
|
||||
}
|
||||
@@ -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 <em>every</em> 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<StockPrice> {
|
||||
|
||||
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);
|
||||
}
|
||||
}
|
||||
@@ -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 + ")";
|
||||
}
|
||||
}
|
||||
@@ -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<Product> 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<Product> 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);
|
||||
}
|
||||
}
|
||||
@@ -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<Employee> employees = OldSchoolComparatorDemo.sampleEmployees();
|
||||
|
||||
List<Employee> viaNamedClass = new ArrayList<>(employees);
|
||||
viaNamedClass.sort(new EmployeeNameComparator());
|
||||
|
||||
List<Employee> 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<Employee> employees = new ArrayList<>(OldSchoolComparatorDemo.sampleEmployees());
|
||||
employees.sort(Comparator.comparing(Employee::getName).thenComparingInt(Employee::getAge));
|
||||
|
||||
List<Employee> 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<Employee> 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<Product> 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<Product> 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<Product> 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<StockPrice> 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<StockPrice> 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<StockPrice> consistentWithEquals =
|
||||
Comparator.comparing(StockPrice::ticker)
|
||||
.thenComparingDouble(StockPrice::price)
|
||||
.thenComparingInt(StockPrice::volume);
|
||||
TreeSet<StockPrice> 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<Ticket> 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<Ticket> buggyResult = new ArrayList<>(input);
|
||||
buggyResult.sort(IntegerOverflowBugDemo::buggyCompare);
|
||||
|
||||
List<Ticket> 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");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user