Code reviewP

- Collapse filterSettings' two identical lookup lambdas into one stringLookup
- Rebuild via SettingsCategory.copy() so new fields aren't silently dropped
- Memoize buildSettingsCatalog with remember(hasPrivateKey, nav, uriHandler);
  onResetMarmot reads isResettingMarmot via rememberUpdatedState to avoid a
  stale-capture, eliminating ~60 allocations per keystroke
This commit is contained in:
davotoula
2026-06-03 09:21:24 +02:00
parent 4446b12972
commit d8e4ada781
3 changed files with 22 additions and 25 deletions
@@ -46,6 +46,7 @@ import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
@@ -97,20 +98,25 @@ fun AllSettingsScreen(
val searchState = rememberTextFieldState()
val query = searchState.text.toString()
// The catalog is structurally stable for the screen's lifetime, so it is rebuilt only when
// an input actually changes — not on every keystroke. `onResetMarmot` reads the volatile
// `isResettingMarmot` through `rememberUpdatedState` so the memoized closure never goes stale.
val onResetMarmot by rememberUpdatedState(newValue = { if (!isResettingMarmot) showResetMarmotDialog = true })
val catalog =
buildSettingsCatalog(
nav = nav,
uriHandler = uriHandler,
hasPrivateKey = hasPrivateKey,
onResetMarmot = { if (!isResettingMarmot) showResetMarmotDialog = true },
)
remember(hasPrivateKey, nav, uriHandler) {
buildSettingsCatalog(
nav = nav,
uriHandler = uriHandler,
hasPrivateKey = hasPrivateKey,
onResetMarmot = { onResetMarmot() },
)
}
val filtered =
filterSettings(
catalog = catalog,
query = query,
titleLookup = { stringRes(context, it) },
keywordsLookup = { stringRes(context, it) },
stringLookup = { stringRes(context, it) },
)
Scaffold(
@@ -56,29 +56,24 @@ data class SettingsCategory(
* Filters [catalog] by [query] (case-insensitive substring over category title + entry title + keywords).
* Blank/whitespace query returns [catalog] unchanged. Categories left with no
* matching entries are dropped. Pure — no Compose/Android — so it is unit-testable;
* string resolution is injected via [titleLookup] / [keywordsLookup].
* string-resource resolution is injected via [stringLookup].
*/
fun filterSettings(
catalog: List<SettingsCategory>,
query: String,
titleLookup: (Int) -> String,
keywordsLookup: (Int) -> String,
stringLookup: (Int) -> String,
): List<SettingsCategory> {
val needle = query.trim().lowercase()
if (needle.isEmpty()) return catalog
return catalog.mapNotNull { category ->
val categoryTitle = titleLookup(category.titleRes)
val categoryTitle = stringLookup(category.titleRes)
val matched =
category.entries.filter { entry ->
val keywords = entry.keywordsRes?.let { keywordsLookup(it) } ?: ""
val haystack = (categoryTitle + " " + titleLookup(entry.titleRes) + " " + keywords).lowercase()
val keywords = entry.keywordsRes?.let { stringLookup(it) } ?: ""
val haystack = (categoryTitle + " " + stringLookup(entry.titleRes) + " " + keywords).lowercase()
haystack.contains(needle)
}
if (matched.isEmpty()) {
null
} else {
SettingsCategory(category.titleRes, category.isDanger, matched)
}
if (matched.isEmpty()) null else category.copy(entries = matched)
}
}
@@ -25,16 +25,13 @@ import org.junit.Assert.assertTrue
import org.junit.Test
class SettingsCatalogFilterTest {
private val titles =
private val strings =
mapOf(
100 to "Account Settings",
200 to "Danger Zone",
1 to "Relay Setup",
2 to "UI Preferences",
3 to "Backup Keys",
)
private val keywords =
mapOf(
20 to "dark mode, theme, font size",
)
@@ -71,8 +68,7 @@ class SettingsCatalogFilterTest {
filterSettings(
catalog = catalog,
query = query,
titleLookup = { titles.getValue(it) },
keywordsLookup = { keywords.getValue(it) },
stringLookup = { strings.getValue(it) },
)
@Test