refactor IDChecker methods to return null instead of exceptions (#13755)

core/src/com/unciv/logic/IdChecker.kt

@@ -24,38 +24,39 @@ import kotlin.math.abs
  */
 object IdChecker {
 
-    fun checkAndReturnPlayerUuid(playerId: String): String {
+    fun checkAndReturnPlayerUuid(playerId: String): String? {
         return checkAndReturnUuiId(playerId, "P")
     }
 
-    fun checkAndReturnGameUuid(gameId: String): String {
+    fun checkAndReturnGameUuid(gameId: String): String? {
         return checkAndReturnUuiId(gameId, "G")
     }
 
-    private fun checkAndReturnUuiId(id: String, prefix: String): String {
+    private fun checkAndReturnUuiId(id: String, prefix: String): String? {
         val trimmedPlayerId = id.trim()
         if (trimmedPlayerId.length == 40) { // length of a UUID (36) with pre- and postfix
-            require(trimmedPlayerId.startsWith(prefix, true)) { "Not a valid ID. Does not start with prefix $prefix" }
+            if (!trimmedPlayerId.startsWith(prefix, true))
+                return null
 
-            val checkDigit = trimmedPlayerId.substring(trimmedPlayerId.lastIndex, trimmedPlayerId.lastIndex +1)
+            val checkDigit = trimmedPlayerId.substring(trimmedPlayerId.lastIndex, trimmedPlayerId.lastIndex + 1)
             // remember, the format is: P-9e37e983-a676-4ecc-800e-ef8ec721a9b9-5
             val shortenedPlayerId = trimmedPlayerId.substring(2, 38)
-            val calculatedCheckDigit = getCheckDigit(shortenedPlayerId).toString()
-            require(calculatedCheckDigit == checkDigit) {
-                "Not a valid ID. Checkdigit invalid."
-            }
+            val calculatedCheckDigit = getCheckDigit(shortenedPlayerId)
+            if (calculatedCheckDigit == null || calculatedCheckDigit.toString() != checkDigit)
+                return null
+
             return shortenedPlayerId
         } else if (trimmedPlayerId.length == 36) {
             return trimmedPlayerId
         }
-        throw IllegalArgumentException("Not a valid ID. Wrong length.")
+        return null
     }
 
 
     /**
      * Adapted from https://wiki.openmrs.org/display/docs/Check+Digit+Algorithm
      */
-    fun getCheckDigit(uuid: String): Int {
+    fun getCheckDigit(uuid: String): Int? {
         // allowable characters within identifier
         @Suppress("SpellCheckingInspection")
         val validChars = "0123456789ABCDEFGHIJKLMNOPQRSTUVYWXZ-"
@@ -73,9 +74,7 @@ object IdChecker {
             val ch = idWithoutCheckdigit[idWithoutCheckdigit.length - i - 1]
 
             // throw exception for invalid characters
-            require(validChars.indexOf(ch) != -1) {
-                "$ch is an invalid character"
-            }
+            if(!validChars.contains(ch)) return null
 
             // our "digit" is calculated using ASCII value - 48
             val digit = ch.code - 48

core/src/com/unciv/ui/screens/multiplayerscreens/AddFriendScreen.kt

@@ -6,14 +6,14 @@ import com.unciv.UncivGame
 import com.unciv.logic.IdChecker
 import com.unciv.logic.multiplayer.FriendList
 import com.unciv.models.translations.tr
-import com.unciv.ui.screens.pickerscreens.PickerScreen
-import com.unciv.ui.popups.ToastPopup
-import com.unciv.ui.components.widgets.UncivTextField
 import com.unciv.ui.components.extensions.enable
-import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
-import java.util.UUID
+import com.unciv.ui.components.input.onClick
+import com.unciv.ui.components.widgets.UncivTextField
+import com.unciv.ui.popups.ToastPopup
+import com.unciv.ui.screens.pickerscreens.PickerScreen
+import com.unciv.utils.isUUID
 
 class AddFriendScreen : PickerScreen() {
     init {
@@ -45,9 +45,7 @@ class AddFriendScreen : PickerScreen() {
         rightSideButton.setText("Add friend".tr())
         rightSideButton.enable()
         rightSideButton.onClick {
-            try {
-                UUID.fromString(IdChecker.checkAndReturnPlayerUuid(playerIDTextField.text))
-            } catch (_: Exception) {
+            if (!(IdChecker.checkAndReturnPlayerUuid(playerIDTextField.text)?.isUUID() ?: false)) {
                 ToastPopup("Player ID is incorrect", this)
                 return@onClick
             }

core/src/com/unciv/ui/screens/multiplayerscreens/AddMultiplayerGameScreen.kt

@@ -5,7 +5,6 @@ import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.Constants
 import com.unciv.logic.IdChecker
 import com.unciv.models.translations.tr
-import com.unciv.ui.components.widgets.UncivTextField
 import com.unciv.ui.components.extensions.enable
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
@@ -13,13 +12,14 @@ import com.unciv.ui.components.input.KeyCharAndCode
 import com.unciv.ui.components.input.keyShortcuts
 import com.unciv.ui.components.input.onActivation
 import com.unciv.ui.components.input.onClick
+import com.unciv.ui.components.widgets.UncivTextField
 import com.unciv.ui.popups.Popup
 import com.unciv.ui.popups.ToastPopup
 import com.unciv.ui.screens.pickerscreens.PickerScreen
 import com.unciv.ui.screens.savescreens.LoadGameScreen
 import com.unciv.utils.Concurrency
+import com.unciv.utils.isUUID
 import com.unciv.utils.launchOnGLThread
-import java.util.UUID
 
 class AddMultiplayerGameScreen(multiplayerScreen: MultiplayerScreen) : PickerScreen() {
     init {
@@ -48,9 +48,7 @@ class AddMultiplayerGameScreen(multiplayerScreen: MultiplayerScreen) : PickerScr
         rightSideButton.enable()
         rightSideButton.keyShortcuts.add(KeyCharAndCode.RETURN)
         rightSideButton.onActivation {
-            try {
-                UUID.fromString(IdChecker.checkAndReturnGameUuid(gameIDTextField.text))
-            } catch (_: Exception) {
+            if (!(IdChecker.checkAndReturnGameUuid(gameIDTextField.text)?.isUUID() ?: false)) {
                 ToastPopup("Invalid game ID!", this)
                 return@onActivation
             }

core/src/com/unciv/ui/screens/multiplayerscreens/EditFriendScreen.kt

@@ -7,15 +7,15 @@ import com.unciv.UncivGame
 import com.unciv.logic.IdChecker
 import com.unciv.logic.multiplayer.FriendList
 import com.unciv.models.translations.tr
-import com.unciv.ui.screens.pickerscreens.PickerScreen
-import com.unciv.ui.popups.ConfirmPopup
-import com.unciv.ui.popups.ToastPopup
-import com.unciv.ui.components.widgets.UncivTextField
 import com.unciv.ui.components.extensions.enable
-import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
-import java.util.UUID
+import com.unciv.ui.components.input.onClick
+import com.unciv.ui.components.widgets.UncivTextField
+import com.unciv.ui.popups.ConfirmPopup
+import com.unciv.ui.popups.ToastPopup
+import com.unciv.ui.screens.pickerscreens.PickerScreen
+import com.unciv.utils.isUUID
 
 class EditFriendScreen(selectedFriend: FriendList.Friend) : PickerScreen() {
     init {
@@ -75,9 +75,7 @@ class EditFriendScreen(selectedFriend: FriendList.Friend) : PickerScreen() {
                 ToastPopup("Player ID already used!", this)
                 return@onClick
             }
-            try {
-                UUID.fromString(IdChecker.checkAndReturnPlayerUuid(playerIDTextField.text))
-            } catch (_: Exception) {
+            if (!(IdChecker.checkAndReturnPlayerUuid(playerIDTextField.text)?.isUUID() ?: false)) {
                 ToastPopup("Player ID is incorrect", this)
                 return@onClick
             }

core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt

@@ -40,11 +40,10 @@ import com.unciv.ui.screens.basescreen.RecreateOnResize
 import com.unciv.ui.screens.pickerscreens.PickerScreen
 import com.unciv.utils.Concurrency
 import com.unciv.utils.Log
+import com.unciv.utils.isUUID
 import com.unciv.utils.launchOnGLThread
 import kotlinx.coroutines.coroutineScope
 import java.net.URI
-import java.net.URL
-import java.util.UUID
 import kotlin.math.floor
 import com.unciv.ui.components.widgets.AutoScrollPane as ScrollPane
 
@@ -157,9 +156,7 @@ class NewGameScreen(
                     else "Couldn't connect to Dropbox!"
 
             for (player in gameSetupInfo.gameParameters.players.filter { it.playerType == PlayerType.Human }) {
-                try {
-                    UUID.fromString(IdChecker.checkAndReturnPlayerUuid(player.playerId))
-                } catch (_: Exception) {
+                if (!(IdChecker.checkAndReturnPlayerUuid(player.playerId)?.isUUID() ?: false)) {
                     return "Invalid player ID!"
                 }
             }

core/src/com/unciv/ui/screens/newgamescreen/PlayerPickerTable.kt

@@ -17,26 +17,26 @@ import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.nation.Nation
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.translations.tr
-import com.unciv.ui.components.input.KeyCharAndCode
-import com.unciv.ui.components.widgets.UncivTextField
-import com.unciv.ui.components.widgets.WrappableLabel
 import com.unciv.ui.components.extensions.darken
 import com.unciv.ui.components.extensions.isEnabled
-import com.unciv.ui.components.input.keyShortcuts
-import com.unciv.ui.components.input.onActivation
-import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.setFontColor
 import com.unciv.ui.components.extensions.surroundWithCircle
 import com.unciv.ui.components.extensions.toCheckBox
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
+import com.unciv.ui.components.input.KeyCharAndCode
+import com.unciv.ui.components.input.keyShortcuts
+import com.unciv.ui.components.input.onActivation
+import com.unciv.ui.components.input.onClick
+import com.unciv.ui.components.widgets.UncivTextField
+import com.unciv.ui.components.widgets.WrappableLabel
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.popups.Popup
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.multiplayerscreens.FriendPickerList
 import com.unciv.ui.screens.pickerscreens.PickerPane
 import com.unciv.ui.screens.pickerscreens.PickerScreen
-import java.util.UUID
+import com.unciv.utils.isUUID
 import com.unciv.ui.components.widgets.AutoScrollPane as ScrollPane
 
 /**
@@ -242,11 +242,10 @@ class PlayerPickerTable(
         add(errorLabel).pad(5f).row()
 
         fun onPlayerIdTextUpdated() {
-            try {
-                UUID.fromString(IdChecker.checkAndReturnPlayerUuid(playerIdTextField.text))
+            if (IdChecker.checkAndReturnPlayerUuid(playerIdTextField.text)?.isUUID() ?: false) {
                 player.playerId = playerIdTextField.text.trim()
                 errorLabel.apply { setText("✔");setFontColor(Color.GREEN) }
-            } catch (_: Exception) {
+            } else {
                 errorLabel.apply { setText("✘");setFontColor(Color.RED) }
             }
         }

tests/src/com/unciv/logic/IdHelperTests.kt

@@ -39,38 +39,41 @@ class IdHelperTests {
     }
 
     @Test
-    fun testIdsSuccess()  {
+    fun testIdsSuccess() {
         val correctString = "2ddb3a34-0699-4126-b7a5-38603e665928"
 
         Assert.assertEquals(correctString, IdChecker.checkAndReturnPlayerUuid(correctString))
-        Assert.assertEquals("c872b8e0-f274-47d4-b761-ce684c5d224c", IdChecker.checkAndReturnGameUuid("c872b8e0-f274-47d4-b761-ce684c5d224c"))
+        Assert.assertEquals(
+            "c872b8e0-f274-47d4-b761-ce684c5d224c",
+            IdChecker.checkAndReturnGameUuid("c872b8e0-f274-47d4-b761-ce684c5d224c")
+        )
 
-        Assert.assertEquals(correctString, IdChecker.checkAndReturnGameUuid("G-" + correctString + "-2"))
-        Assert.assertEquals(correctString, IdChecker.checkAndReturnPlayerUuid("P-" + correctString + "-2"))
+        Assert.assertEquals(correctString, IdChecker.checkAndReturnGameUuid("G-$correctString-2"))
+        Assert.assertEquals(correctString, IdChecker.checkAndReturnPlayerUuid("P-$correctString-2"))
     }
 
-    @Test(expected = IllegalArgumentException::class)
+    @Test // too short
     fun testIdFailure1() {
-        IdChecker.checkAndReturnGameUuid("2ddb3a34-0699-4126-b7a5-38603e66592") // too short
+        Assert.assertNull(IdChecker.checkAndReturnGameUuid("2ddb3a34-0699-4126-b7a5-38603e66592"))
     }
 
-    @Test(expected = IllegalArgumentException::class)
+    @Test // wrong prefix
     fun testIdFailure2() {
-        IdChecker.checkAndReturnGameUuid("P-2ddb3a34-0699-4126-b7a5-38603e665928-2") // wrong prefix
+        Assert.assertNull(IdChecker.checkAndReturnGameUuid("P-2ddb3a34-0699-4126-b7a5-38603e665928-2"))
     }
 
-    @Test(expected = IllegalArgumentException::class)
+    @Test // wrong prefix
     fun testIdFailure3() {
-        IdChecker.checkAndReturnPlayerUuid("G-2ddb3a34-0699-4126-b7a5-38603e665928-2") // wrong prefix
+        Assert.assertNull(IdChecker.checkAndReturnPlayerUuid("G-2ddb3a34-0699-4126-b7a5-38603e665928-2"))
     }
 
-    @Test(expected = IllegalArgumentException::class)
+    @Test // changed checkDigit
     fun testIdFailure4() {
-        IdChecker.checkAndReturnGameUuid("G-2ddb3a34-0699-4126-b7a5-38603e665928-3") // changed checkDigit
+        Assert.assertNull(IdChecker.checkAndReturnGameUuid("G-2ddb3a34-0699-4126-b7a5-38603e665928-3"))
     }
 
-    @Test(expected = IllegalArgumentException::class)
+    @Test // changed uuid without changing checkdigit
     fun testIdFailure5() {
-        IdChecker.checkAndReturnGameUuid("G-2ddb3a34-0699-4126-b7a5-48603e665928-2") // changed uuid without changing checkdigit
+        Assert.assertNull(IdChecker.checkAndReturnGameUuid("G-2ddb3a34-0699-4126-b7a5-48603e665928-2"))
     }
 }