Skip to content

Commit b3b8b54

Browse files
Arthur-MilchiorArthur-Milchior
committed
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 427da06 commit b3b8b54

File tree

22 files changed

+255
-159
lines changed

22 files changed

+255
-159
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
@@ -929,7 +929,7 @@ class ContentProviderTest : InstrumentedTest() {
929929
assertNotNull("Check that the deck we received actually exists", deck)
930930
assertEquals(
931931
"Check that the received deck has the correct name",
932-
deck.getString("name"),
932+
deck.name,
933933
deckName,
934934
)
935935
}
@@ -963,7 +963,7 @@ class ContentProviderTest : InstrumentedTest() {
963963
)
964964
assertEquals(
965965
"Check that received deck name equals real deck name",
966-
realDeck.getString("name"),
966+
realDeck.name,
967967
returnedDeckName,
968968
)
969969
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2303,7 +2303,7 @@ open class DeckPicker :
23032303
} catch (e: RuntimeException) {
23042304
Timber.e(e, "RuntimeException setting time remaining")
23052305
}
2306-
val current = withCol { decks.current().optLong("id") }
2306+
val current = withCol { decks.current().id }
23072307
if (focusedDeck != current) {
23082308
scrollDecklistToDeck(current)
23092309
focusedDeck = current

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

Lines changed: 38 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ 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
@@ -59,92 +60,90 @@ class FilteredDeckOptions :
5960
"{'search'=\"\", 'steps'=\"1 10 20\", 'order'=0}",
6061
)
6162

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

6567
override fun cacheValues() {
6668
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)
69+
deck.firstFilter.apply {
70+
values["search"] = search
71+
values["limit"] = limit
72+
values["order"] = order.toString()
7873
}
79-
val delays = deck.optJSONArray("delays")
74+
deck.secondFilter?.apply {
75+
values["search_2"] = search
76+
values["limit_2"] = limit
77+
values["order_2"] = order.toString()
78+
}
79+
val delays = deck.delays
8080
if (delays != null) {
8181
values["steps"] = convertFromJSON(delays)
8282
values["stepsOn"] = java.lang.Boolean.toString(true)
8383
} else {
8484
values["steps"] = "1 10"
8585
values["stepsOn"] = java.lang.Boolean.toString(false)
8686
}
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")
87+
values["resched"] = java.lang.Boolean.toString(deck.resched)
88+
values["previewAgainSecs"] = deck.previewAgainSecs
89+
values["previewHardSecs"] = deck.previewHardSecs
90+
values["previewGoodSecs"] = deck.previewGoodSecs
9191
}
9292

93-
inner class Editor : AppCompatPreferenceActivity<FilteredDeckOptions.DeckPreferenceHack>.AbstractPreferenceHack.Editor() {
93+
inner class Editor : AppCompatPreferenceActivity<DeckPreferenceHack>.AbstractPreferenceHack.Editor() {
9494
override fun commit(): Boolean {
9595
Timber.d("commit() changes back to database")
9696
for ((key, value) in update.valueSet()) {
9797
Timber.i("Change value for key '%s': %s", key, value)
98-
val ar = deck.getJSONArray("terms")
99-
if (pref.secondFilter) {
98+
deck.secondFilter?.apply {
10099
when (key) {
101100
"search_2" -> {
102-
ar.getJSONArray(1).put(0, value)
101+
search = value as String
103102
}
104103
"limit_2" -> {
105-
ar.getJSONArray(1).put(1, value)
104+
limit = value as String
106105
}
107106
"order_2" -> {
108-
ar.getJSONArray(1).put(2, (value as String).toInt())
107+
order = (value as String).toInt()
109108
}
110109
}
111110
}
112111
when (key) {
113112
"search" -> {
114-
ar.getJSONArray(0).put(0, value)
113+
deck.firstFilter.search = value as String
115114
}
116115

117116
"limit" -> {
118-
ar.getJSONArray(0).put(1, value)
117+
deck.firstFilter.limit = value as String
119118
}
120119
"order" -> {
121-
ar.getJSONArray(0).put(2, (value as String).toInt())
120+
deck.firstFilter.order = (value as String).toInt()
122121
}
123122
"resched" -> {
124-
deck.put("resched", value)
123+
deck.resched = value as Boolean
125124
}
126125
"previewAgainSecs" -> {
127-
deck.put("previewAgainSecs", value)
126+
deck.previewAgainSecs = value as String
128127
}
129128
"previewHardSecs" -> {
130-
deck.put("previewHardSecs", value)
129+
deck.previewHardSecs = value as String
131130
}
132131
"previewGoodSecs" -> {
133-
deck.put("previewGoodSecs", value)
132+
deck.previewGoodSecs = value as String
134133
}
135134
"stepsOn" -> {
136135
val on = value as Boolean
137136
if (on) {
138137
val steps = convertToJSON(values["steps"]!!)
139138
if (steps!!.length() > 0) {
140-
deck.put("delays", steps)
139+
deck.delays = steps
141140
}
142141
} else {
143-
deck.put("delays", JSONObject.NULL)
142+
deck.delays = null
144143
}
145144
}
146145
"steps" -> {
147-
deck.put("delays", convertToJSON((value as String)))
146+
deck.delays = convertToJSON((value as String))
148147
}
149148
"preset" -> {
150149
val i: Int = (value as String).toInt()
@@ -210,7 +209,7 @@ class FilteredDeckOptions :
210209
return
211210
}
212211
val extras = intent.extras
213-
deck = if (extras != null && extras.containsKey("did")) {
212+
deck = if (extras?.containsKey("did") == true) {
214213
col.decks.get(extras.getLong("did"))
215214
} else {
216215
null
@@ -238,7 +237,7 @@ class FilteredDeckOptions :
238237
if (title.contains("XXX")) {
239238
title =
240239
try {
241-
title.replace("XXX", deck.getString("name"))
240+
title.replace("XXX", deck.name)
242241
} catch (e: JSONException) {
243242
Timber.w(e)
244243
title.replace("XXX", "???")
@@ -282,7 +281,7 @@ class FilteredDeckOptions :
282281
if (prefChanged) {
283282
// Rebuild the filtered deck if a setting has changed
284283
try {
285-
col.sched.rebuildDyn(deck.getLong("id"))
284+
col.sched.rebuildDyn(deck.id)
286285
} catch (e: JSONException) {
287286
Timber.e(e)
288287
}
@@ -355,15 +354,14 @@ class FilteredDeckOptions :
355354
return@OnPreferenceChangeListener true
356355
}
357356
if (!newValue) {
358-
deck.getJSONArray("terms").remove(1)
357+
deck.removeSecondFilter()
359358
secondFilter.isEnabled = false
360359
} else {
361360
secondFilter.isEnabled = true
362361
/**Link to the defaults used in AnkiDesktop
363362
* <https://github.com/ankitects/anki/blob/1b15069b248a8f86f9bd4b3c66a9bfeab8dfb2b8/qt/aqt/filtered_deck.py#L148-L149>
364363
*/
365-
val narr = JSONArray(listOf("", 20, 5))
366-
deck.getJSONArray("terms").put(1, narr)
364+
deck.secondFilter = Term(JSONArray(listOf("", 20, 5)))
367365
val newOrderPrefSecond = findPreference("order_2") as ListPreference
368366
newOrderPrefSecond.value = "5"
369367
}

‎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.mid = newId
26532653
getColUnsafe.decks.save(currentDeck)
26542654

26552655
// Update deck

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

Lines changed: 4 additions & 5 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()) {
@@ -600,7 +599,7 @@ class StudyOptionsFragment :
600599
if (isDynamic) {
601600
resources.getString(R.string.dyn_deck_desc)
602601
} else {
603-
col.decks.current().description
602+
col.decks.current().desc
604603
}
605604
if (desc.isNotEmpty()) {
606605
textDeckDescription.text = formatDescription(desc)

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,13 @@ 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
3534
import timber.log.Timber
3635

3736
/**
38-
* Allows a user to edit the [deck description][description]
37+
* Allows a user to edit the [deck description][desc]
3938
*
4039
* This is visible on [StudyOptionsFragment]
4140
*/
@@ -116,11 +115,11 @@ class EditDeckDescriptionDialog : DialogFragment() {
116115
}
117116
}
118117

119-
private suspend fun getDescription() = withCol { decks.get(deckId)!!.description }
118+
private suspend fun getDescription() = withCol { decks.get(deckId)!!.desc }
120119

121120
private suspend fun setDescription(value: String) {
122121
Timber.i("updating deck description")
123-
withCol { decks.update(deckId) { description = value } }
122+
withCol { decks.update(deckId) { desc = value } }
124123
}
125124

126125
companion object {

‎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
@@ -57,7 +57,6 @@ import com.ichi2.anki.withProgress
5757
import com.ichi2.annotations.NeedsTest
5858
import com.ichi2.libanki.Collection
5959
import com.ichi2.libanki.Consts
60-
import com.ichi2.libanki.Consts.DynPriority
6160
import com.ichi2.libanki.Deck
6261
import com.ichi2.libanki.DeckId
6362
import com.ichi2.libanki.undoableOp
@@ -71,7 +70,6 @@ import com.ichi2.utils.positiveButton
7170
import com.ichi2.utils.textAsIntOrNull
7271
import com.ichi2.utils.title
7372
import net.ankiweb.rsdroid.exceptions.BackendDeckIsFilteredException
74-
import org.json.JSONObject
7573
import timber.log.Timber
7674

7775
/**
@@ -418,10 +416,10 @@ class CustomStudyDialog(
418416
} else {
419417
Timber.i("Emptying dynamic deck '%s' for custom study", customStudyDeck)
420418
// safe to empty
421-
collection.sched.emptyDyn(cur.getLong("id"))
419+
collection.sched.emptyDyn(cur.id)
422420
// reuse; don't delete as it may have children
423421
dyn = cur
424-
decks.select(cur.getLong("id"))
422+
decks.select(cur.id)
425423
}
426424
} else {
427425
Timber.i("Creating Dynamic Deck '%s' for custom study", customStudyDeck)
@@ -434,13 +432,13 @@ class CustomStudyDialog(
434432
}
435433
}
436434
// and then set various options
437-
dyn.put("delays", JSONObject.NULL)
438-
val ar = dyn.getJSONArray("terms")
439-
ar.getJSONArray(0).put(0, """deck:"$deckToStudyName" terms[0]""")
440-
ar.getJSONArray(0).put(1, terms[1])
441-
@DynPriority val priority = terms[2] as Int
442-
ar.getJSONArray(0).put(2, priority)
443-
dyn.put("resched", true)
435+
dyn.delays = null
436+
dyn.firstFilter.apply {
437+
search = """deck:"$deckToStudyName" terms[0]"""
438+
limit = terms[1] as String
439+
order = terms[2] as Int
440+
}
441+
dyn.resched = true
444442
// Rebuild the filtered deck
445443
Timber.i("Rebuilding Custom Study Deck")
446444
// PERF: Should be in background

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

Lines changed: 2 additions & 3 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.desc = deckDesc
983982
}
984983
} catch (e: JSONException) {
985984
Timber.e(e, "Could not set a field of new deck %s", deckName)
@@ -1239,7 +1238,7 @@ class CardContentProvider : ContentProvider() {
12391238
}
12401239
FlashCardsContract.Deck.DECK_DYN -> rb.add(col.decks.isFiltered(id))
12411240
FlashCardsContract.Deck.DECK_DESC -> {
1242-
val desc = col.decks.current().description
1241+
val desc = col.decks.current().desc
12431242
rb.add(desc)
12441243
}
12451244
}

‎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)