Specific id that were int #18393

Arthur-Milchior · 2025-05-30T00:00:55Z

This commit ensure some values that are ID being int or long get a more specific type when possible, for the sake of typesafety

lukstbit

Tests are broken...

lukstbit · 2025-05-30T06:53:24Z

AnkiDroid/src/main/java/com/ichi2/utils/AlertDialogFacade.kt

+/**
+ * @param inputType an [InputType]
+ */
+enum class AlertType(


Nope, I'm not ok with this change:

the input types are well known by anyone that has work even a bit with the Android EditText and it makes no sense to replace them with an enum just for type safety

the name is strange and a bit unrelated

at the call site, editText.inputType = InputType.TYPE_CLASS_NUMBER is better than editText.inputType = AlertType.Number.inputType

this class was intended just as a thin layer over AlertDialog, there's no need to introduce this kind of customization IMO

I'd be happy with removing AlertDialogFacade

The point was to make a transition away from afollestad.material-dialogs easier, which we've accomplished.

I personally still like the class, but the consensus was to remove it

lukstbit · 2025-05-30T08:55:04Z

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

@@ -107,23 +107,36 @@ class CardBrowserMySearchesDialog : AnalyticsDialogFragment() {
        }
    }

+    enum class CardBrowserMySearches(


As it's scoped to CardBrowserMySearchesDialog already I would use the name: SavedSearchesType.

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

lukstbit · 2025-05-30T09:07:59Z

AnkiDroid/src/main/java/com/ichi2/anki/reviewer/ActionButtonStatus.kt

+    /**
+     * @param actionEnum - How the item should display.
+     */
+    enum class ShowAsAction(


This change doesn't look too valuable as the enum is used only in this class. And outside isn't valuable as well because it has one item that doesn't correspond to Android behavior(so it's no a perfect model).

This makes clearer what the type is. Sadly, this can't be done everywhere when the Int is a Android enum encoded as an interface

david-allison · 2025-06-15T17:33:11Z

AnkiDroid/src/main/java/com/ichi2/anki/browser/CardOrNoteId.kt

 @JvmInline
 value class CardOrNoteId(
-    val cardOrNoteId: Long,
+    val cardOrNoteId: CardOrNoteIdUnderlyingType,


revert - no purpose

david-allison · 2025-06-15T17:33:28Z

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

-        val type = requireArguments().getInt("type")
-        if (type == CARD_BROWSER_MY_SEARCHES_TYPE_LIST) {
-            savedFilters = requireArguments().getSerializableCompat("savedFilters")
+        val type = SavedSearchesType.fromCode(requireArguments().getInt(SAVED_TYPE_KEY))


Why not serialise/deserialise the enum directly?

david-allison · 2025-06-15T17:34:14Z

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/help/HelpItem.kt

@@ -18,6 +18,8 @@ import androidx.annotation.DrawableRes
 import androidx.annotation.StringRes
 import kotlinx.parcelize.Parcelize

+typealias HelpItemId = Long


make it private, I don't think it serves much purpose

david-allison · 2025-06-15T17:34:41Z

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/tags/TagsDialog.kt

@@ -183,9 +183,9 @@ class TagsDialog : AnalyticsDialogFragment {
                }
                check(0)
            }
-        selectedOption = radioButtonIdToCardState(optionsGroup.checkedRadioButtonId)
+        selectedOption = CardStateFilter.fromCode(optionsGroup.checkedRadioButtonId)


Revert - we should remove this functionality

david-allison · 2025-06-15T17:35:34Z

AnkiDroid/src/main/java/com/ichi2/anki/pages/ImageOcclusion.kt

@@ -122,7 +124,7 @@ class ImageOcclusion : PageFragment(R.layout.image_occlusion) {
        fun getIntent(
            context: Context,
            kind: String,
-            noteOrNotetypeId: Long,
+            noteOrNotetypeId: NoteOrNoteTypeId,


revert? Or Document, or strongly type as a union

david-allison · 2025-06-15T17:36:13Z

AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt

@@ -42,6 +42,8 @@ import com.ichi2.widget.setRecurringAlarm
 import kotlinx.coroutines.launch
 import timber.log.Timber

+typealias AppWidgetId = Int


make this a value class

github-actions · 2025-07-06T09:37:54Z

Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

lukstbit requested changes May 30, 2025

View reviewed changes

lukstbit added the Needs Author Reply Waiting for a reply from the original author label May 30, 2025

github-actions bot added the Has Conflicts label Jun 2, 2025

Arthur-Milchior force-pushed the type_enum branch from 03252b6 to 7d5179e Compare June 15, 2025 13:25

Arthur-Milchior removed Needs Author Reply Waiting for a reply from the original author Has Conflicts labels Jun 15, 2025

Arthur-Milchior added 3 commits June 15, 2025 15:34

NF: some types are changed from int to enum

0112d3d

This makes clearer what the type is. Sadly, this can't be done everywhere when the Int is a Android enum encoded as an interface

NF: replace id: Long by more specific type

3f3980d

NF: Replace "id: Int" by more specific type

da02fab

Arthur-Milchior force-pushed the type_enum branch from 7d5179e to da02fab Compare June 15, 2025 13:35

david-allison added Needs Review Needs Author Reply Waiting for a reply from the original author and removed Needs Review labels Jun 17, 2025

david-allison requested changes Jun 17, 2025

View reviewed changes

github-actions bot added the Has Conflicts label Jun 22, 2025

github-actions bot added the Stale label Jul 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Specific id that were int #18393

Specific id that were int #18393

Arthur-Milchior commented May 30, 2025

Uh oh!

lukstbit left a comment

Uh oh!

lukstbit May 30, 2025

Uh oh!

david-allison Jun 3, 2025 •

edited

Loading

Uh oh!

lukstbit May 30, 2025

Uh oh!

Uh oh!

lukstbit May 30, 2025

Uh oh!

david-allison Jun 15, 2025

Uh oh!

david-allison Jun 15, 2025

Uh oh!

david-allison Jun 15, 2025

Uh oh!

david-allison Jun 15, 2025

Uh oh!

david-allison Jun 15, 2025

Uh oh!

david-allison Jun 15, 2025

Uh oh!

github-actions bot commented Jul 6, 2025

Uh oh!

Uh oh!

Uh oh!

Specific id that were int #18393

Are you sure you want to change the base?

Specific id that were int #18393

Conversation

Arthur-Milchior commented May 30, 2025

Uh oh!

lukstbit left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-allison Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 6, 2025

Uh oh!

Uh oh!

david-allison Jun 3, 2025 •

edited

Loading