Remove inputted keys separated by spaces from map and output the resulting map, including alias recognition

Question

I'm looking to integrate Kotlin idioms (including code style) and maybe better performance. I wonder if there's a better way to do line 17 (the str.split(' ') one).

// 0L: Empty 1L: Inactive
// Maps have been truncated
// Full map has 2 empties, 5 inactives, and 15 18-digit numbers (IDs)
val game = mutableMapOf(
    "warzone" to 123456789012345678,
    "csgo" to 1L,
    "dead" to 0L
)
// Full alias map has 16 entries
val alias = mapOf(
    "cod" to "warzone",
    "dbd" to "dead"
)
fun main() {
    var str = readln()
    while (str.isNotEmpty()) {
        str.split(' ').map { it.lowercase() }.forEach {
            if (game.contains(it))
                game.remove(it)
            else if (alias.contains(it))
                game.remove(alias[it])
            else
                System.err.println("Unrecognized key: $it")
        }
        for ((k, v) in game) {
            if (v != 0L && v != 1L) print("<@$v>: ")
            else if (v == 1L) print("__Inactive__: ")
            else print("__Empty__: ")
            println("*$k*")
        }
        str = readln()
    }
}

Answer 1

Split more

Separate data transfer into more atomic parts to make it more streamlined. That will make it easier to read and use.

In your case - first you filter your keys that you are interested in and then afterwards you do operations on them (remove keys from the map):

    str.split(' ').map { it.lowercase() }.mapNotNull {
        if (game.contains(it)) {
            it
        } else if (alias.contains(it)) {
            alias[it]
        } else {
            System.err.println("Unrecognized key: $it")
            null
        }
    }.forEach {keyToRemove ->
        game.remove(keyToRemove)
    }

Prepare your data

Your input data is not perfect to use for your task, you can transform them beforehand to make your life easier later. Depending on amount data this can make your code run faster (since you some calculations only once at the beginning).

You can prepare all possible keys before your loop so that you don't have to have this "if" condition and merge all the keys together:

    val keysToBeRemoved = alias + game.map { it.key to it.key }
    str.split(' ').map { it.lowercase() }.mapNotNull {word ->
        keysToBeRemoved[word].also {
            if (it == null) {
                System.err.println("Unrecognized key: $word")
            }
        }
    }.forEach {keyToRemove ->
        game.remove(keyToRemove)
    }

That is much better.

Handle errors better

Depending on your usecase your error handling can be fine, but in my experience it's usually better to fail fast on invalid data input. This changes functionality of your code so maybe you don't want that:

    str.split(' ').map { it.lowercase() }.map { word ->
        keysToBeRemoved[word] ?: error("Unrecognized key: $word")
    }.forEach { keyToRemove ->
        game.remove(keyToRemove)
    }

Enforce immutability

You are modifying original game mutable. That means your code can run only once. For a script it's fine, but overall it's better to have immutable data and then filter and transform it as you need:

//immutable
    val game = mapOf(
        "warzone" to 123456789012345678,
        "csgo" to 1L,
        "dead" to 0L
    )

...

    val keysToRemove: Set<String> = str.split(' ').map { it.lowercase() }.map { word ->
        keysToBeRemoved[word] ?: error("Unrecognized key: $word")
    }.toSet()
    val gameWithRemovedKeys = game.filter { it.key !in keysToRemove }

Use functions

Now that everything is nicely separated, you can put these into functions:

fun filterGames(words: List<String>): Map<String, Long> {
    val keysToBeRemoved = alias + game.map { it.key to it.key }
    val keysToRemove: Set<String> = words.map { it.lowercase() }.map { word ->
        keysToBeRemoved[word] ?: error("Unrecognized key: $word")
    }.toSet()
    return game.filter { it.key !in keysToRemove }
}

fun printGames(gameWithRemovedKeys: Map<String, Long>) {
    for ((k, v) in gameWithRemovedKeys) {
        if (v != 0L && v != 1L) print("<@$v>: ")
        else if (v == 1L) print("__Inactive__: ")
        else print("__Empty__: ")
        println("*$k*")
    }
}

Your main then becomes this:

fun main() {
    var str = readln()
    while (str.isNotEmpty()) {
        printGames(filterGames(str.split(' ')))
        str = readln()
    }
}

Use classes to represent your data

This one is optional and can be overkill for scripts, but if you are doing something sophisticated and the data is important later, it should be done. I would define this class for your usecase:

This could be done better using interfaces and sealed classes or more sophisticated enums to ensure statically that it's impossible to have inactive game with ID for example, but I am not sure that's what you want.

It conveniently cleans up print function by implementing toString (or it could be different method so that toString output isn't too bloated.

Further changes would be more and more opinionated so I will top here. This is the code so far:

// 0L: Empty 1L: Inactive
// Maps have been truncated
// Full map has 2 empties, 5 inactives, and 15 18-digit numbers (IDs)
val game = mapOf(
    "warzone" to 123456789012345678,
    "csgo" to 1L,
    "dead" to 0L
)

// Full alias map has 16 entries
val alias = mapOf(
    "cod" to "warzone",
    "dbd" to "dead"
)

data class Game(
    val id: Long,
    val name: String,
    val state: State = State.fromId(id),
) {
    override fun toString(): String {
        return when (state) {
            State.ACTIVE -> "<@$id>: *${name}*"
            State.INACTIVE -> "__Inactive__: *${name}*"
            State.DEAD -> "__Empty__: *${name}*"
        }
    }
}

fun filterGames(words: List<String>): List<Game> {
    val keysToBeRemoved = alias + game.map { it.key to it.key }
    val keysToRemove: Set<String> = words.map { it.lowercase() }.map { word ->
        keysToBeRemoved[word] ?: error("Unrecognized key: $word")
    }.toSet()
    return game.filter { it.key !in keysToRemove }.map {
        Game(id = it.value, name = it.key)
    }
}

fun printGames(gameWithRemovedKeys: List<Game>) {
    for (game in gameWithRemovedKeys) {
        println(game)
    }
}

enum class State {
    INACTIVE, DEAD, ACTIVE;

    companion object {
        fun fromId(id: Long): State {
            return when (id) {
                0L -> INACTIVE
                1L -> DEAD
                else -> ACTIVE
            }
        }
    }
}

fun main() {
    var str = readln()
    while (str.isNotEmpty()) {
        printGames(filterGames(str.split(' ')))
        str = readln()
    }
}