Simple Kotlin Bingo Class

Question

This is a simple class in Kotlin that handles three basic Bingo-related functionalities: Specifying the numbers, generating a card, and calling one random element. Additionally, it should support an empty constructor, and support Discord Emoji output, in which there should be no spaces between any elements. I personally think drivers that use this class should do their own error handling.

I'd like suggestions on how to make this more in line with Kotlin standards, e.g. idioms, code style, whether or not I should delegate error handling to drivers, alternate/more readable approaches to certain subtasks...

Edit: I have changed the code to add an additonal constructor that specifies the delimiting char between elements.

Edit on constructor: The constructor splits the input string along the spl val and puts the first element into delim. For the rest, if the first character isn't : (meaning the input isn't Discord Emoji), the elements are sorted by length alone before being padded with spaces so 1. all elements are of the same length 2. all elements have a trailing space. Then, the header is initialized with the first 5 elements of the Column enum using the same padding conditions. For Discord Emoji input, the header is initialized similarly but with regional indicators.

import kotlin.random.Random

class Bingo() {
    var num = (1..75).map { if (it < 10) " $it " else "$it " }
    var cur = num.associateBy({ num.indexOf(it) }, { it }).toMutableMap()
    var delim = " ∅ "
    var max = 2
    var header = "B  I  N  G  O"

    enum class Column {
        B,
        I,
        N,
        G,
        O,
        Empty
    }

    /**
     * It is expected that the number of elements ≥ 25 and is divisible by 5
     */
    constructor(nums: String, spl: Char) : this() {
        val rem = nums.substringAfter(spl).split(spl).toMutableList()
        while (rem.contains("")) rem.remove("")
        while (rem.size % 5 != 0) rem.remove(rem.random())
        if (rem.size < 25) return
        num = rem; delim = nums.substringBefore(spl)
        if (num[0][0] != ':') {
            num = num.sortedBy { it.length }.toMutableList()
            max = num.last().length
            num = num.map { " ".repeat(max - it.length) + it + " " }
            delim = " ".repeat(max - delim.length) + delim + " "
            header = ""
            for (i in (0..4)) header += "" + Column.values()[i] + " ".repeat(max)
        } else {
            header = ":regional_indicator_" + Column.values()[0].toString().lowercase()
            //Reg. indicators often turn into flags without spaces
            for (i in (1..4)) header += " :regional_indicator_" + Column.values()[i].toString().lowercase()
        }
        cur = num.associateBy({ num.indexOf(it) }, { it }).toMutableMap()
    }

    constructor(nums: String): this(nums, '@')

    fun card(): Array<Array<String>> {
        val ret = Array(5) { Array(5) { "" } }
        var index: Int
        val done = mutableListOf<Int>()
        for (i in (1..5))
            for (j in (1..5)) {
                if (i == j && j == 3) {
                    ret[2][2] = delim
                    continue
                }
                do {
                    index = Random.nextInt(num.size / 5 * (j - 1), num.size / 5 * j)
                } while (done.contains(index))
                ret[i - 1][j - 1] = num[index]
                done.add(index)
            }
        return ret
    }

    fun call(): Pair<Column, String> {
        if (cur.isEmpty()) return Pair(Column.Empty, "")
        var index: Int
        do index = Random.nextInt(num.size) while (cur[index] == null)
        val list = cur[index]!!.trim()
        cur.remove(index)
        return Pair(Column.values()[index / (num.size / 5)], list)
    }
}
```

Answer 1

Let's start with the basics:

Use meaningful names. Names such as num, cur, spl and rem are short but they don't make the code more readable. The name list is especially confusing since its type is a String.

Never use ; to separate statements on one line as it makes the code cluttered.

Don't use mutable names (variables that can be reassigned) without a good reason, as this practice is error-prone and mutability-related errors are often very hard to debug. Always use read-only names (val) where you can. In this particular program every single var can be replaced with a val, consider doing it as an exercise.

---

Let's go back to the top.

Kotlin supports primary constructors. Always use this feature to its fullest by making the largest constructor primary. Any other constructors will call the primary (or each other) with some defaults:

class A(
    val foo: String,
    val bar: Int,
) {
    constructor() : this(
        DEFAULT_FOO,
        DEFAULT_BAR,
    )
}

The second constructor takes 20 lines to parse some nums string. This code is very hard to comprehend since there is no format for this string specified anywhere. There should be a constructor with everything the class really needs (num, cur, etc..) and another one that takes the string you want to parse.

---

There are default parameters in kotlin: instead of

constructor(nums: String, spl: Char) : ...

constructor(nums: String): this(nums, '@')

write

constructor(nums: String, spl: Char = '@') : ...

---

fun card(): Array<Array<String>>

Avoid using arrays, they have a lot of disadvantages compared to the modern List interface (e.g. mutability and invariance).

---

Try using kotlin standard library more, it makes the code more readable and concise when used wisely. E.g. this

fun card(): Array<Array<String>> {
    val ret = Array(5) { Array(5) { "" } }
    var index: Int
    val done = mutableListOf<Int>()
    for (i in (1..5))
        for (j in (1..5)) {
            if (i == j && j == 3) {
                ret[2][2] = delim
                continue
            }
            do {
                index = Random.nextInt(num.size / 5 * (j - 1), num.size / 5 * j)
            } while (done.contains(index))
            ret[i - 1][j - 1] = num[index]
            done.add(index)
        }
    return ret
}

can be rewritten as this:

fun test(): List<List<String>> = num
    .shuffled()
    .slice(0 until 25)
    .toMutableList()
    .also { it[12] = delim }
    .chunked(5)