Skip to content

Commit b7ba1ae

Browse files
NF: Transform Deck into a class value
The first interest of this change is that we are not deep cloning all decks when creating a Deck object which should save a little bit of time, especially loading time on collection with a lot of decks. Instead, we have a better typing at zero runtime cost. The second interest is that we are certain that the fact that the Deck is implemented as a JSONObject is hidden from the developer using this class. Instead, they can only use the provided extension, or add the one they needs. While most changes are relatively trivial, moving the implementation from everywhere in the code to Deck.kt, there are some extra changes that were needed. I moved `getLongOrNull` to PythonExtension given that it implements the behavior of a Python function we want to reuse, this seems more consistent. As value class can't be lateinit, I hardcoded AppCompatPreferenceActivity as being a getter for the backing field `_deck: Deck?`. I added in testutils/JsonUtils helper method that accepts a json holder, and a string representing the expected json, to simplify the use of those methods. The code used sometime `optLong("id")` and sometime `getLong("id")`. There seems to be no reason for the difference between both use case, and just be random choice from the developer. Anyway, if a deck lacks an id, any place using `getLong("id")` would have caused the code to crash, so replacing the `optLong` by `getLong` would, in the worst case, move the time at which the user is asked to clean their database. There seems to be a single key we sometime set to JSONObject.NULL, it was `delays`. For the sake of typing, the property `delays` in `deck` is thus of type `JSONArray?`. When the value `null` is assigned to this property, `NULL` is saved in the object, and reciprocally. There is no need to check whether `deck` has "empty" key before removing it. It's not even more efficient.
1 parent 8f7b4c3 commit b7ba1ae

File tree

22 files changed

+505
-154
lines changed

22 files changed

+505
-154
lines changed

AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -930,7 +930,7 @@ class ContentProviderTest : InstrumentedTest() {
930930
assertNotNull("Check that the deck we received actually exists", deck)
931931
assertEquals(
932932
"Check that the received deck has the correct name",
933-
deck.getString("name"),
933+
deck.name,
934934
deckName,
935935
)
936936
}
@@ -964,7 +964,7 @@ class ContentProviderTest : InstrumentedTest() {
964964
)
965965
assertEquals(
966966
"Check that received deck name equals real deck name",
967-
realDeck.getString("name"),
967+
realDeck.name,
968968
returnedDeckName,
969969
)
970970
}

AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,7 @@ open class DeckPicker :
993993
override fun onQueryTextChange(newText: String): Boolean {
994994
val adapter = recyclerView.adapter as DeckAdapter
995995
launchCatchingTask {
996-
val selectedDeckId = withCol { decks.current().getLong("id") }
996+
val selectedDeckId = withCol { decks.current().id }
997997
dueTree?.let {
998998
adapter.submit(
999999
data = it.filterAndFlatten(newText),
@@ -2333,7 +2333,7 @@ open class DeckPicker :
23332333
deckListAdapter.submit(
23342334
data = tree.filterAndFlatten(currentFilter),
23352335
hasSubDecks = tree.children.any { it.children.any() },
2336-
currentDeckId = withCol { decks.current().getLong("id") },
2336+
currentDeckId = withCol { decks.current().id },
23372337
)
23382338

23392339
// Set the "x due" subtitle
@@ -2347,7 +2347,7 @@ open class DeckPicker :
23472347
}.onFailure {
23482348
Timber.w(it, "Failed to set the due count as the subtitle in the toolbar")
23492349
}
2350-
val current = withCol { decks.current().optLong("id") }
2350+
val current = withCol { decks.current().id }
23512351
if (viewModel.focusedDeck != current) {
23522352
scrollDecklistToDeck(current)
23532353
viewModel.focusedDeck = current

AnkiDroid/src/main/java/com/ichi2/anki/FilteredDeckOptions.kt

Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ import androidx.core.content.edit
3030
import com.ichi2.anki.analytics.UsageAnalytics
3131
import com.ichi2.annotations.NeedsTest
3232
import com.ichi2.libanki.Collection
33+
import com.ichi2.libanki.Deck.Term
3334
import com.ichi2.preferences.StepsPreference.Companion.convertFromJSON
3435
import com.ichi2.preferences.StepsPreference.Companion.convertToJSON
3536
import com.ichi2.themes.Themes
3637
import com.ichi2.ui.AppCompatPreferenceActivity
3738
import com.ichi2.utils.stringIterable
38-
import org.json.JSONArray
3939
import org.json.JSONException
4040
import org.json.JSONObject
4141
import timber.log.Timber
@@ -59,92 +59,90 @@ class FilteredDeckOptions :
5959
"{'search'=\"\", 'steps'=\"1 10 20\", 'order'=0}",
6060
)
6161

62-
inner class DeckPreferenceHack : AppCompatPreferenceActivity<FilteredDeckOptions.DeckPreferenceHack>.AbstractPreferenceHack() {
63-
var secondFilter = false
62+
inner class DeckPreferenceHack : AppCompatPreferenceActivity<DeckPreferenceHack>.AbstractPreferenceHack() {
63+
val hasSecondFilter: Boolean
64+
get() = deck.secondFilter != null
6465

6566
override fun cacheValues() {
6667
Timber.d("cacheValues()")
67-
val ar = deck.getJSONArray("terms").getJSONArray(0)
68-
secondFilter = deck.getJSONArray("terms").length() > 1
69-
val ar2: JSONArray?
70-
values["search"] = ar.getString(0)
71-
values["limit"] = ar.getString(1)
72-
values["order"] = ar.getString(2)
73-
if (secondFilter) {
74-
ar2 = deck.getJSONArray("terms").getJSONArray(1)
75-
values["search_2"] = ar2.getString(0)
76-
values["limit_2"] = ar2.getString(1)
77-
values["order_2"] = ar2.getString(2)
68+
deck.firstFilter.apply {
69+
values["search"] = search
70+
values["limit"] = limit
71+
values["order"] = order.toString()
7872
}
79-
val delays = deck.optJSONArray("delays")
73+
deck.secondFilter?.apply {
74+
values["search_2"] = search
75+
values["limit_2"] = limit
76+
values["order_2"] = order.toString()
77+
}
78+
val delays = deck.delays
8079
if (delays != null) {
8180
values["steps"] = convertFromJSON(delays)
8281
values["stepsOn"] = java.lang.Boolean.toString(true)
8382
} else {
8483
values["steps"] = "1 10"
8584
values["stepsOn"] = java.lang.Boolean.toString(false)
8685
}
87-
values["resched"] = java.lang.Boolean.toString(deck.getBoolean("resched"))
88-
values["previewAgainSecs"] = deck.getString("previewAgainSecs")
89-
values["previewHardSecs"] = deck.getString("previewHardSecs")
90-
values["previewGoodSecs"] = deck.getString("previewGoodSecs")
86+
values["resched"] = java.lang.Boolean.toString(deck.resched)
87+
values["previewAgainSecs"] = deck.previewAgainSecs
88+
values["previewHardSecs"] = deck.previewHardSecs
89+
values["previewGoodSecs"] = deck.previewGoodSecs
9190
}
9291

93-
inner class Editor : AppCompatPreferenceActivity<FilteredDeckOptions.DeckPreferenceHack>.AbstractPreferenceHack.Editor() {
92+
inner class Editor : AppCompatPreferenceActivity<DeckPreferenceHack>.AbstractPreferenceHack.Editor() {
9493
override fun commit(): Boolean {
9594
Timber.d("commit() changes back to database")
9695
for ((key, value) in update.valueSet()) {
9796
Timber.i("Change value for key '%s': %s", key, value)
98-
val ar = deck.getJSONArray("terms")
99-
if (pref.secondFilter) {
97+
deck.secondFilter?.apply {
10098
when (key) {
10199
"search_2" -> {
102-
ar.getJSONArray(1).put(0, value)
100+
search = value as String
103101
}
104102
"limit_2" -> {
105-
ar.getJSONArray(1).put(1, value)
103+
limit = value as String
106104
}
107105
"order_2" -> {
108-
ar.getJSONArray(1).put(2, (value as String).toInt())
106+
order = (value as String).toInt()
109107
}
110108
}
111109
}
112110
when (key) {
113111
"search" -> {
114-
ar.getJSONArray(0).put(0, value)
112+
deck.firstFilter.search = value as String
115113
}
116114

117115
"limit" -> {
118-
ar.getJSONArray(0).put(1, value)
116+
deck.firstFilter.limit = value as String
119117
}
120118
"order" -> {
121-
ar.getJSONArray(0).put(2, (value as String).toInt())
119+
deck.firstFilter.order = (value as String).toInt()
122120
}
123121
"resched" -> {
124-
deck.put("resched", value)
122+
deck.resched = value as Boolean
125123
}
126124
"previewAgainSecs" -> {
127-
deck.put("previewAgainSecs", value)
125+
deck.previewAgainSecs = value as String
128126
}
129127
"previewHardSecs" -> {
130-
deck.put("previewHardSecs", value)
128+
deck.previewHardSecs = value as String
131129
}
132130
"previewGoodSecs" -> {
133-
deck.put("previewGoodSecs", value)
131+
deck.previewGoodSecs = value as String
134132
}
135133
"stepsOn" -> {
136134
val on = value as Boolean
137135
if (on) {
138136
val steps = convertToJSON(values["steps"]!!)
139137
if (steps!!.length() > 0) {
140-
deck.put("delays", steps)
138+
deck.delays = steps
141139
}
142140
} else {
143-
deck.put("delays", JSONObject.NULL)
141+
deck.delays = null
144142
}
145143
}
146144
"steps" -> {
147-
deck.put("delays", convertToJSON((value as String)))
145+
deck.delays = convertToJSON((value as String))
148146
}
149147
"preset" -> {
150148
val i: Int = (value as String).toInt()
@@ -210,7 +208,7 @@ class FilteredDeckOptions :
210208
return
211209
}
212210
val extras = intent.extras
213-
deck = if (extras != null && extras.containsKey("did")) {
211+
deck = if (extras?.containsKey("did") == true) {
214212
col.decks.get(extras.getLong("did"))
215213
} else {
216214
null
@@ -238,7 +236,7 @@ class FilteredDeckOptions :
238236
if (title.contains("XXX")) {
239237
title =
240238
try {
241-
title.replace("XXX", deck.getString("name"))
239+
title.replace("XXX", deck.name)
242240
} catch (e: JSONException) {
243241
Timber.w(e)
244242
title.replace("XXX", "???")
@@ -282,7 +280,7 @@ class FilteredDeckOptions :
282280
if (prefChanged) {
283281
// Rebuild the filtered deck if a setting has changed
284282
try {
285-
col.sched.rebuildDyn(deck.getLong("id"))
283+
col.sched.rebuildDyn(deck.id)
286284
} catch (e: JSONException) {
287285
Timber.e(e)
288286
}
@@ -336,7 +334,7 @@ class FilteredDeckOptions :
336334
newOrderPref.value = pref.getString("order", "0")
337335
newOrderPrefSecond.setEntries(R.array.cram_deck_conf_order_labels)
338336
newOrderPrefSecond.setEntryValues(R.array.cram_deck_conf_order_values)
339-
if (pref.secondFilter) {
337+
if (pref.hasSecondFilter) {
340338
newOrderPrefSecond.value = pref.getString("order_2", "5")
341339
}
342340
}
@@ -345,7 +343,7 @@ class FilteredDeckOptions :
345343
private fun setupSecondFilterListener() {
346344
val secondFilterSign = findPreference("filterSecond") as CheckBoxPreference
347345
val secondFilter = findPreference("secondFilter") as PreferenceCategory
348-
if (pref.secondFilter) {
346+
if (pref.hasSecondFilter) {
349347
secondFilter.isEnabled = true
350348
secondFilterSign.isChecked = true
351349
}
@@ -355,15 +353,14 @@ class FilteredDeckOptions :
355353
return@OnPreferenceChangeListener true
356354
}
357355
if (!newValue) {
358-
deck.getJSONArray("terms").remove(1)
356+
deck.secondFilter = null
359357
secondFilter.isEnabled = false
360358
} else {
361359
secondFilter.isEnabled = true
362360
/**Link to the defaults used in AnkiDesktop
363361
* <https://github.com/ankitects/anki/blob/1b15069b248a8f86f9bd4b3c66a9bfeab8dfb2b8/qt/aqt/filtered_deck.py#L148-L149>
364362
*/
365-
val narr = JSONArray(listOf("", 20, 5))
366-
deck.getJSONArray("terms").put(1, narr)
363+
deck.secondFilter = Term("", 20.toString(), 5)
367364
val newOrderPrefSecond = findPreference("order_2") as ListPreference
368365
newOrderPrefSecond.value = "5"
369366
}

AnkiDroid/src/main/java/com/ichi2/anki/NoteEditor.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2649,7 +2649,7 @@ class NoteEditor :
26492649

26502650
getColUnsafe.notetypes.setCurrent(model)
26512651
val currentDeck = getColUnsafe.decks.current()
2652-
currentDeck.put("mid", newId)
2652+
currentDeck.noteTypeId = newId
26532653
getColUnsafe.decks.save(currentDeck)
26542654

26552655
// Update deck

AnkiDroid/src/main/java/com/ichi2/anki/StudyOptionsFragment.kt

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ import com.ichi2.anki.CollectionManager.withCol
4343
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog
4444
import com.ichi2.anki.snackbar.showSnackbar
4545
import com.ichi2.anki.ui.internationalization.toSentenceCase
46-
import com.ichi2.anki.utils.ext.description
4746
import com.ichi2.anki.utils.ext.showDialogFragment
4847
import com.ichi2.libanki.ChangeManager
4948
import com.ichi2.libanki.Collection
@@ -422,8 +421,8 @@ class StudyOptionsFragment :
422421
if (loadWithDeckOptions) {
423422
loadWithDeckOptions = false
424423
val deck = col!!.decks.current()
425-
if (deck.isFiltered && deck.has("empty")) {
426-
deck.remove("empty")
424+
if (deck.isFiltered) {
425+
deck.removeEmpty()
427426
}
428427
launchCatchingTask { rebuildCram() }
429428
} else {
@@ -553,7 +552,7 @@ class StudyOptionsFragment :
553552
// Set the deck name
554553
val deck = col.decks.current()
555554
// Main deck name
556-
val fullName = deck.getString("name")
555+
val fullName = deck.name
557556
val name = Decks.path(fullName)
558557
val nameBuilder = StringBuilder()
559558
if (name.isNotEmpty()) {

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/EditDeckDescriptionDialog.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import com.ichi2.anki.CollectionManager.withCol
2828
import com.ichi2.anki.R
2929
import com.ichi2.anki.StudyOptionsFragment
3030
import com.ichi2.anki.launchCatchingTask
31-
import com.ichi2.anki.utils.ext.description
3231
import com.ichi2.anki.utils.ext.update
3332
import com.ichi2.libanki.DeckId
3433
import com.ichi2.themes.Themes

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ import com.ichi2.anki.withProgress
6363
import com.ichi2.annotations.NeedsTest
6464
import com.ichi2.libanki.Collection
6565
import com.ichi2.libanki.Consts
66-
import com.ichi2.libanki.Consts.DynPriority
6766
import com.ichi2.libanki.Deck
6867
import com.ichi2.libanki.DeckId
6968
import com.ichi2.libanki.undoableOp
@@ -78,7 +77,6 @@ import com.ichi2.utils.setPaddingRelative
7877
import com.ichi2.utils.textAsIntOrNull
7978
import com.ichi2.utils.title
8079
import net.ankiweb.rsdroid.exceptions.BackendDeckIsFilteredException
81-
import org.json.JSONObject
8280
import timber.log.Timber
8381

8482
/**
@@ -439,10 +437,10 @@ class CustomStudyDialog(
439437
} else {
440438
Timber.i("Emptying dynamic deck '%s' for custom study", customStudyDeck)
441439
// safe to empty
442-
collection.sched.emptyDyn(cur.getLong("id"))
440+
collection.sched.emptyDyn(cur.id)
443441
// reuse; don't delete as it may have children
444442
dyn = cur
445-
decks.select(cur.getLong("id"))
443+
decks.select(cur.id)
446444
}
447445
} else {
448446
Timber.i("Creating Dynamic Deck '%s' for custom study", customStudyDeck)
@@ -455,13 +453,13 @@ class CustomStudyDialog(
455453
}
456454
}
457455
// and then set various options
458-
dyn.put("delays", JSONObject.NULL)
459-
val ar = dyn.getJSONArray("terms")
460-
ar.getJSONArray(0).put(0, """deck:"$deckToStudyName" terms[0]""")
461-
ar.getJSONArray(0).put(1, terms[1])
462-
@DynPriority val priority = terms[2] as Int
463-
ar.getJSONArray(0).put(2, priority)
464-
dyn.put("resched", true)
456+
dyn.delays = null
457+
dyn.firstFilter.apply {
458+
search = """deck:"$deckToStudyName" terms[0]"""
459+
limit = terms[1] as String
460+
order = terms[2] as Int
461+
}
462+
dyn.resched = true
465463
// Rebuild the filtered deck
466464
Timber.i("Rebuilding Custom Study Deck")
467465
// PERF: Should be in background

AnkiDroid/src/main/java/com/ichi2/anki/provider/CardContentProvider.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import com.ichi2.anki.CollectionManager
3434
import com.ichi2.anki.CrashReportService
3535
import com.ichi2.anki.Ease
3636
import com.ichi2.anki.FlashCardsContract
37-
import com.ichi2.anki.utils.ext.description
3837
import com.ichi2.libanki.Card
3938
import com.ichi2.libanki.CardTemplate
4039
import com.ichi2.libanki.Collection
@@ -979,7 +978,7 @@ class CardContentProvider : ContentProvider() {
979978
try {
980979
val deckDesc = values.getAsString(FlashCardsContract.Deck.DECK_DESC)
981980
if (deckDesc != null) {
982-
deck.put("desc", deckDesc)
981+
deck.description = deckDesc
983982
}
984983
} catch (e: JSONException) {
985984
Timber.e(e, "Could not set a field of new deck %s", deckName)

AnkiDroid/src/main/java/com/ichi2/anki/services/ReminderService.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ class ReminderService : BroadcastReceiver() {
149149
for (node in dues) {
150150
val deck: Deck? = col.decks.get(node.did)
151151
// Dynamic deck has no "conf", so are not added here.
152-
if (deck != null && deck.optLong("conf") == dConfId) {
152+
if (deck != null && deck.conf == dConfId) {
153153
decks.add(node)
154154
}
155155
}

AnkiDroid/src/main/java/com/ichi2/anki/utils/ext/Deck.kt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,6 @@ import com.ichi2.libanki.Deck
2020
import com.ichi2.libanki.DeckId
2121
import com.ichi2.libanki.Decks
2222

23-
var Deck.description: String
24-
get() = optString("desc", "")
25-
set(value) {
26-
put("desc", value)
27-
}
28-
2923
fun Decks.update(
3024
did: DeckId,
3125
block: Deck.() -> Unit,

0 commit comments

Comments
 (0)