mirror of
https://github.com/praktimarc/kst4contest.git
synced 2026-10-19 13:53:01 +02:00
fix: sort the AP column numerically instead of by its text (#87)
The "AP [minutes / pot%]" column of the station list and of the directed
messages had no comparator, so DataTableState fell back to the rendered
text and "13 (75%)" sorted above "3 (50%)".
AirplaneCell now owns a shared comparator, AirplaneCell.order, that both
column definitions use. The sort key is the first aeroplane of the cell,
which is the one the operator reads and the one the colour marks:
ascending by its arriving minutes, then by the better potential first for
equal minutes. A null info (nothing computed) and an empty list ("nil")
carry Int.MAX_VALUE, so they sort after every real aeroplane in ascending
order, consistent with the qrb and qtf sentinels. The second aeroplane of
a cell does not influence the order. Cell text, colours, column ids,
titles and weights are unchanged, as are DataTableState and the sort
cycle.
Fixes #87
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
456957808d
commit
ab28216280
8 files changed
+150
No files matched your search
@@ -1,6 +1,7 @@
|
||||
package kst4contest.view.compose
|
||||
|
||||
import androidx.compose.ui.graphics.Color
|
||||
import kst4contest.model.AirPlane
|
||||
import kst4contest.model.AirPlaneReflectionInfo
|
||||
|
||||
/**
|
||||
@@ -34,6 +35,27 @@ object AirplaneCell {
|
||||
*/
|
||||
private const val NOTHING_COMING = "nil"
|
||||
|
||||
// ---- the order ----------------------------------------------------------
|
||||
|
||||
/** Unknown or no aeroplane sorts after every real one. */
|
||||
private const val NO_PLANE = Int.MAX_VALUE
|
||||
|
||||
/** The aeroplane the cell shows first; the parser already put the best one there. */
|
||||
private fun first(info: AirPlaneReflectionInfo?): AirPlane? = info?.risingAirplanes?.firstOrNull()
|
||||
|
||||
/**
|
||||
* Ascending: the minutes the operator reads first, then the better potential first.
|
||||
*
|
||||
* Issue #87: without a comparator the table fell back to the rendered text, which put
|
||||
* "13 (75%)" above "3 (50%)". Only the first aeroplane counts, as it is the one the
|
||||
* operator reads and the one the colour marks; the second is in the cell for context.
|
||||
* Empty and "nil" cells carry [NO_PLANE]: like qrb and qtf, an absent value gets a
|
||||
* sentinel instead of a null check, here the maximum so the cell sorts last ascending.
|
||||
*/
|
||||
val order: Comparator<AirPlaneReflectionInfo?> =
|
||||
compareBy<AirPlaneReflectionInfo?> { first(it)?.arrivingDurationMinutes ?: NO_PLANE }
|
||||
.thenByDescending { first(it)?.potential ?: 0 }
|
||||
|
||||
/** `.table-cell-100PercentAP`, identical in both stylesheets: a certain path. */
|
||||
val PURPLE_100 = Color(0xFFF98AFF)
|
||||
|
||||
|
||||
@@ -57,6 +57,7 @@ object DirectedMessageColumns {
|
||||
title = "AP [minutes / pot%]",
|
||||
value = { AirplaneCell.format(it.sender?.airPlaneReflectInfo) },
|
||||
accent = { _, rendered -> AirplaneCell.accentFor(rendered) },
|
||||
comparator = compareBy(AirplaneCell.order) { it.sender?.airPlaneReflectInfo },
|
||||
weight = 1.4f,
|
||||
),
|
||||
DataColumn(
|
||||
|
||||
@@ -55,6 +55,7 @@ object StationColumns {
|
||||
title = "AP [minutes / pot%]",
|
||||
value = { AirplaneCell.format(it.airPlaneReflectInfo) },
|
||||
accent = { _, rendered -> AirplaneCell.accentFor(rendered) },
|
||||
comparator = compareBy(AirplaneCell.order) { it.airPlaneReflectInfo },
|
||||
weight = 1.4f,
|
||||
))
|
||||
add(DataColumn(id = "tropo", title = "Tropo", value = tropoOf, weight = 0.7f))
|
||||
|
||||
@@ -4,6 +4,7 @@ import kst4contest.model.AirPlane
|
||||
import kst4contest.model.AirPlaneReflectionInfo
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.Assertions.assertNull
|
||||
import org.junit.jupiter.api.Assertions.assertSame
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
/**
|
||||
@@ -97,6 +98,70 @@ class AirplaneCellTest {
|
||||
assertEquals(AirplaneCell.RED_75, AirplaneCell.accentFor("7 (50%) / 12 (75%)")?.foreground)
|
||||
}
|
||||
|
||||
// ---- the order ----------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Issue #87: the column was sorted by its rendered text, so "13 (75%)" came before
|
||||
* "3 (50%)". These are the thirteen cells of the screenshot in the issue, in the order
|
||||
* they were shown; sorted, the minutes have to read as numbers.
|
||||
*/
|
||||
@Test
|
||||
fun `the minutes sort as numbers, not as text`() {
|
||||
val sorted = screenshotCells().sortedWith(AirplaneCell.order)
|
||||
|
||||
assertEquals(
|
||||
listOf(1, 1, 3, 3, 6, 6, 8, 8, 9, 9, 13, 14, 19),
|
||||
sorted.map { it.risingAirplanes.first().arrivingDurationMinutes },
|
||||
)
|
||||
}
|
||||
|
||||
/** Same minutes: the better potential first, which is also the one the colour marks. */
|
||||
@Test
|
||||
fun `equal minutes put the better potential first`() {
|
||||
val sorted = screenshotCells().sortedWith(AirplaneCell.order)
|
||||
|
||||
assertEquals(
|
||||
listOf("3 (75%)", "3 (50%)"),
|
||||
sorted.map { AirplaneCell.format(it) }.filter { it.startsWith("3 ") },
|
||||
"the screenshot had 3 (50%) above 3 (75%); the order is not just stable",
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* An empty cell (nothing computed) and "nil" (no aeroplane coming) both carry the
|
||||
* maximum sentinel, so ascending puts them after the last real aeroplane.
|
||||
*/
|
||||
@Test
|
||||
fun `no aeroplane sorts after the last real one`() {
|
||||
val nothingComputed: AirPlaneReflectionInfo? = null
|
||||
val nothingComing = reflectionInfo()
|
||||
val cells = listOf(nothingComputed, nothingComing) + screenshotCells()
|
||||
|
||||
val sorted = cells.sortedWith(AirplaneCell.order)
|
||||
|
||||
assertEquals(19, sorted[sorted.size - 3]?.risingAirplanes?.first()?.arrivingDurationMinutes)
|
||||
assertSame(nothingComputed, sorted[sorted.size - 2])
|
||||
assertSame(nothingComing, sorted[sorted.size - 1])
|
||||
}
|
||||
|
||||
/** Only the first aeroplane, the one the operator reads first, decides. */
|
||||
@Test
|
||||
fun `the second aeroplane of a cell does not influence the order`() {
|
||||
val nineThenOne = reflectionInfo(plane(9, 75), plane(1, 100))
|
||||
val threeThenTwenty = reflectionInfo(plane(3, 50), plane(20, 50))
|
||||
|
||||
val sorted = listOf(nineThenOne, threeThenTwenty).sortedWith(AirplaneCell.order)
|
||||
|
||||
assertSame(threeThenTwenty, sorted[0])
|
||||
assertSame(nineThenOne, sorted[1])
|
||||
}
|
||||
|
||||
/** The AP column of the issue's screenshot, top to bottom: (minutes, potential). */
|
||||
private fun screenshotCells(): List<AirPlaneReflectionInfo> = listOf(
|
||||
1 to 75, 1 to 75, 13 to 75, 14 to 50, 19 to 50, 3 to 50, 3 to 75,
|
||||
6 to 75, 6 to 75, 8 to 75, 8 to 75, 9 to 75, 9 to 75,
|
||||
).map { (minutes, potential) -> reflectionInfo(plane(minutes, potential)) }
|
||||
|
||||
private fun reflectionInfo(vararg planes: AirPlane): AirPlaneReflectionInfo =
|
||||
AirPlaneReflectionInfo().apply { risingAirplanes = planes.toMutableList() }
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@ import kst4contest.model.ChatCategory
|
||||
import kst4contest.model.ChatMember
|
||||
import kst4contest.model.ChatMessage
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.Assertions.assertNotNull
|
||||
import org.junit.jupiter.api.Assertions.assertTrue
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
@@ -98,6 +99,33 @@ class DirectedMessageColumnsTest {
|
||||
assertEquals("7 (85%)", cell("airscout", message(sender = sender)))
|
||||
}
|
||||
|
||||
/**
|
||||
* Issue #87: the AP column had no comparator and fell back to its text, so "13 (75%)"
|
||||
* sorted before "9 (75%)". Same shape as DataTableStateTest's "a column with a
|
||||
* comparator sorts by it, not by its text", with the real columns.
|
||||
*/
|
||||
@Test
|
||||
fun `airscout sorts numerically`() {
|
||||
assertNotNull(columns.single { it.id == "airscout" }.comparator, "the AP column sorts by its own key")
|
||||
|
||||
val table = DataTableState(columns = columns, rowKey = RowKeys.byReference<ChatMessage>())
|
||||
table.replaceRows(listOf(
|
||||
message(sender = memberWithAeroplane("DL13ABC", minutes = 13)),
|
||||
message(sender = memberWithAeroplane("DL9ABC", minutes = 9)),
|
||||
))
|
||||
|
||||
table.toggleSort("airscout")
|
||||
|
||||
assertEquals(listOf("DL9ABC", "DL13ABC"), table.visibleRows.map { it.sender?.callSign },
|
||||
"9 before 13 numerically; by text it would be the other way round")
|
||||
}
|
||||
|
||||
private fun memberWithAeroplane(call: String, minutes: Int): ChatMember = member(call).apply {
|
||||
airPlaneReflectInfo = AirPlaneReflectionInfo().apply {
|
||||
risingAirplanes = mutableListOf(AirPlane().apply { arrivingDurationMinutes = minutes; potential = 75 })
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the category names the chat the message arrived in`() {
|
||||
val message = message(sender = member("DL0ABC")).apply { chatCategory = ChatCategory(2) }
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
package kst4contest.view.compose
|
||||
|
||||
import kst4contest.model.AirPlane
|
||||
import kst4contest.model.AirPlaneReflectionInfo
|
||||
import kst4contest.model.ChatMember
|
||||
import org.junit.jupiter.api.Test
|
||||
import org.junit.jupiter.api.Assertions.assertDoesNotThrow
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.Assertions.assertNotNull
|
||||
import org.junit.jupiter.api.Assertions.assertNull
|
||||
import org.junit.jupiter.api.Assertions.assertTrue
|
||||
|
||||
@@ -120,6 +123,32 @@ class StationColumnsTest {
|
||||
assertEquals("", columns().first { it.id == "not-qrv" }.value(ChatMember()))
|
||||
}
|
||||
|
||||
/**
|
||||
* Issue #87: the AP column had no comparator and fell back to its text, so "13 (75%)"
|
||||
* sorted before "9 (75%)". Same shape as DataTableStateTest's "a column with a
|
||||
* comparator sorts by it, not by its text", with the real columns.
|
||||
*/
|
||||
@Test
|
||||
fun `airscout sorts numerically`() {
|
||||
val columns = columns()
|
||||
assertNotNull(columns.first { it.id == "airscout" }.comparator, "the AP column sorts by its own key")
|
||||
|
||||
val table = DataTableState(columns = columns, rowKey = RowKeys.byValue<ChatMember> { it.callSign.orEmpty() })
|
||||
table.replaceRows(listOf(stationWithAeroplane("DL13ABC", minutes = 13), stationWithAeroplane("DL9ABC", minutes = 9)))
|
||||
|
||||
table.toggleSort("airscout")
|
||||
|
||||
assertEquals(listOf("DL9ABC", "DL13ABC"), table.visibleRows.map { it.callSign },
|
||||
"9 before 13 numerically; by text it would be the other way round")
|
||||
}
|
||||
|
||||
private fun stationWithAeroplane(call: String, minutes: Int): ChatMember = ChatMember().apply {
|
||||
setCallSign(call)
|
||||
airPlaneReflectInfo = AirPlaneReflectionInfo().apply {
|
||||
risingAirplanes = mutableListOf(AirPlane().apply { arrivingDurationMinutes = minutes; potential = 75 })
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `tropo and score come from outside the station`() {
|
||||
val columns = StationColumns.all(
|
||||
|
||||
Reference in new issue
Block a user