Remove non idiomatic usages of lists

3 files changed, 42 insertions, 20 deletions

diff --git a/backend/src/main/kotlin/org/luxons/sevenwonders/lobby/Lobby.kt b/backend/src/main/kotlin/org/luxons/sevenwonders/lobby/Lobby.kt
index 173ced64..8498ce85 100644
--- a/backend/src/main/kotlin/org/luxons/sevenwonders/lobby/Lobby.kt
+++ b/backend/src/main/kotlin/org/luxons/sevenwonders/lobby/Lobby.kt
@@ -3,7 +3,6 @@ package org.luxons.sevenwonders.lobby
 import org.luxons.sevenwonders.game.Game
 import org.luxons.sevenwonders.game.api.CustomizableSettings
 import org.luxons.sevenwonders.game.data.GameDefinition
-import java.util.ArrayList
 
 enum class State {
     LOBBY, PLAYING
@@ -15,7 +14,7 @@ class Lobby(
     var owner: Player,
     @field:Transient private val gameDefinition: GameDefinition
 ) {
-    private var players: MutableList<Player> = ArrayList(gameDefinition.maxPlayers)
+    private val players: MutableList<Player> = ArrayList(gameDefinition.maxPlayers)
 
     var settings: CustomizableSettings = CustomizableSettings()
 
@@ -64,7 +63,11 @@ class Lobby(
 
     @Synchronized
     fun reorderPlayers(orderedUsernames: List<String>) {
-        players = orderedUsernames.map { find(it) }.toMutableList()
+        val usernames = players.map { it.username }
+        if (orderedUsernames.toSet() != usernames.toSet()) {
+            throw PlayerListMismatchException(orderedUsernames)
+        }
+        players.sortBy { orderedUsernames.indexOf(it.username) }
     }
 
     private fun find(username: String): Player =
@@ -98,8 +101,11 @@ class Lobby(
         IllegalStateException("Minimum $min players required to start a game")
 
     internal class PlayerNameAlreadyUsedException(displayName: String, gameName: String) :
-        IllegalStateException("Name '$displayName' is already used by a player in game '$gameName'")
+        IllegalArgumentException("Name '$displayName' is already used by a player in game '$gameName'")
 
     internal class UnknownPlayerException(username: String) :
-        IllegalStateException("Unknown player '$username'")
+        IllegalArgumentException("Unknown player '$username'")
+
+    internal class PlayerListMismatchException(usernames: List<String>) :
+        IllegalArgumentException("Newly ordered usernames $usernames don't match the current player list")
 }
diff --git a/backend/src/test/kotlin/org/luxons/sevenwonders/controllers/LobbyControllerTest.kt b/backend/src/test/kotlin/org/luxons/sevenwonders/controllers/LobbyControllerTest.kt
index 2d37b0f4..6e417970 100644
--- a/backend/src/test/kotlin/org/luxons/sevenwonders/controllers/LobbyControllerTest.kt
+++ b/backend/src/test/kotlin/org/luxons/sevenwonders/controllers/LobbyControllerTest.kt
@@ -15,7 +15,6 @@ import org.luxons.sevenwonders.repositories.LobbyRepository
 import org.luxons.sevenwonders.repositories.PlayerNotFoundException
 import org.luxons.sevenwonders.repositories.PlayerRepository
 import org.luxons.sevenwonders.test.mockSimpMessagingTemplate
-import java.util.Arrays
 import java.util.HashMap
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
@@ -99,10 +98,10 @@ class LobbyControllerTest {
         val player3 = addPlayer(lobby, "testuser3")
         val player4 = addPlayer(lobby, "testuser4")
 
-        val players = Arrays.asList(player, player2, player3, player4)
+        val players = listOf(player, player2, player3, player4)
         assertEquals(players, lobby.getPlayers())
 
-        val reorderedPlayers = Arrays.asList(player3, player, player2, player4)
+        val reorderedPlayers = listOf(player3, player, player2, player4)
         val playerNames = reorderedPlayers.map { it.username }
         val reorderPlayersAction = ReorderPlayersAction(playerNames)
 
@@ -120,7 +119,7 @@ class LobbyControllerTest {
         val player2 = addPlayer(lobby, "testuser2")
         val player3 = addPlayer(lobby, "testuser3")
 
-        val reorderedPlayers = Arrays.asList(player3, player, player2)
+        val reorderedPlayers = listOf(player3, player, player2)
         val playerNames = reorderedPlayers.map { it.username }
         val reorderPlayersAction = ReorderPlayersAction(playerNames)
 
diff --git a/backend/src/test/kotlin/org/luxons/sevenwonders/lobby/LobbyTest.kt b/backend/src/test/kotlin/org/luxons/sevenwonders/lobby/LobbyTest.kt
index 85c89595..98e154da 100644
--- a/backend/src/test/kotlin/org/luxons/sevenwonders/lobby/LobbyTest.kt
+++ b/backend/src/test/kotlin/org/luxons/sevenwonders/lobby/LobbyTest.kt
@@ -13,11 +13,11 @@ import org.junit.runner.RunWith
 import org.luxons.sevenwonders.game.api.CustomizableSettings
 import org.luxons.sevenwonders.game.data.GameDefinition
 import org.luxons.sevenwonders.lobby.Lobby.GameAlreadyStartedException
+import org.luxons.sevenwonders.lobby.Lobby.PlayerListMismatchException
 import org.luxons.sevenwonders.lobby.Lobby.PlayerNameAlreadyUsedException
 import org.luxons.sevenwonders.lobby.Lobby.PlayerOverflowException
 import org.luxons.sevenwonders.lobby.Lobby.PlayerUnderflowException
 import org.luxons.sevenwonders.lobby.Lobby.UnknownPlayerException
-import java.util.Arrays
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
@@ -150,21 +150,38 @@ class LobbyTest {
         lobby.addPlayer(player1)
         lobby.addPlayer(player2)
         lobby.addPlayer(player3)
-        lobby.reorderPlayers(Arrays.asList("testuser3", "testuser1", "testuser2"))
-        assertEquals("testuser3", lobby.getPlayers()[0].username)
-        assertEquals("testuser1", lobby.getPlayers()[1].username)
-        assertEquals("testuser2", lobby.getPlayers()[2].username)
+
+        val reorderedUsernames = listOf("testuser3", "gameowner", "testuser1", "testuser2")
+        lobby.reorderPlayers(reorderedUsernames)
+
+        assertEquals(reorderedUsernames, lobby.getPlayers().map { it.username })
     }
 
-    @Test(expected = UnknownPlayerException::class)
+    @Test(expected = PlayerListMismatchException::class)
     fun reorderPlayers_failsOnUnknownPlayer() {
         val player1 = Player("testuser1", "Test User 1")
         val player2 = Player("testuser2", "Test User 2")
-        val player3 = Player("testuser3", "Test User 3")
         lobby.addPlayer(player1)
         lobby.addPlayer(player2)
-        lobby.addPlayer(player3)
-        lobby.reorderPlayers(Arrays.asList("unknown", "testuser1", "testuser2"))
+        lobby.reorderPlayers(listOf("unknown", "testuser2", "gameowner"))
+    }
+
+    @Test(expected = PlayerListMismatchException::class)
+    fun reorderPlayers_failsOnExtraPlayer() {
+        val player1 = Player("testuser1", "Test User 1")
+        val player2 = Player("testuser2", "Test User 2")
+        lobby.addPlayer(player1)
+        lobby.addPlayer(player2)
+        lobby.reorderPlayers(listOf("testuser2", "onemore", "testuser1", "gameowner"))
+    }
+
+    @Test(expected = PlayerListMismatchException::class)
+    fun reorderPlayers_failsOnMissingPlayer() {
+        val player1 = Player("testuser1", "Test User 1")
+        val player2 = Player("testuser2", "Test User 2")
+        lobby.addPlayer(player1)
+        lobby.addPlayer(player2)
+        lobby.reorderPlayers(listOf("testuser2", "gameowner"))
     }
 
     @Theory
@@ -203,11 +220,11 @@ class LobbyTest {
 
     @Test
     fun startGame_switchesState() {
-        assertTrue(lobby.state === State.LOBBY)
+        assertEquals(State.LOBBY, lobby.state)
         // there is already the owner
         addPlayers(gameDefinition.minPlayers - 1)
         lobby.startGame()
-        assertTrue(lobby.state === State.PLAYING)
+        assertEquals(State.PLAYING, lobby.state)
     }
 
     @Test