fakechitor - tennis-scoreboard-kotlin.md

https://github.com/fakechitor/tennis-scoreboard-kotlin
Ревью от Ильи @coderilya

Общее впечатление спорное, с одной стороны проект на котлине, но с другой я не понимаю какие фишки синтаксиса использовались) Плюс приложение совсем не threadsafe (потокобезопасно). Да, в тз это не требуется, но я не знаю где ещё в роудмапе практиковаться хотя бы с той же cuncurrent hash map (или скорее в твоем случае atomic, synchronized | lock). Постораюсь в ревью не акцентировать на таких деталях (в разговоре можем обсудить), лучше поделюсь как импрувнуть понимание синтаксиса котлина (не претендую на скилл мастера и если что отвечаю в чате/личке).

Что советовал бы рефакторить:

1. У нас приложение без IoC и DI, а значит надо создавать классы синглтонами (если хотим threadsafe - потокобезопасно). Гайд для котлина https://www.baeldung.com/kotlin/singleton-classes

2. Диблирование кода в catch часто,

https://github.com/fakechitor/tennis-scoreboard-kotlin/blob/2eab20acd9805e4bc31fb5c881207db1c515cccd/src/main/kotlin/controller/MatchScoreController.kt#L45C2-L59C10

можно пофиксить например вот так

try {
    // do some work
} catch (ex: Exception) {
    when(ex) {
        is IllegalAccessException, is IndexOutOfBoundsException -> {
            // handle those above
        }
        else -> throw ex
    }
}

Но вообще у тебя в коде надо убрать перехват IllegalArgumentException (и в целом много желания всё перехватить, тем более даже в ванильной джаве это можно сделать частично через фильтры).

3. Посмотри как работает дефолтный конструктор в котлине + data class https://kotlinlang.org/docs/classes.html#constructors https://www.baeldung.com/kotlin/jpa

4. Мне не нравится подход с классом Validator Validation, нам он объектом и не не особо нужен) Лучше посмотри в сторону exstantion. Можно было бы их использовать на String и валидировать на ней, если это делаешь вручную (только не делать новые методы слишком громоздкими, потому что это всё таки ещё должна остаться строка). https://kotlinlang.org/docs/extensions.html

5. Очень плотные методы и есть шум, который котлин может убрать в коде, например:

https://github.com/fakechitor/tennis-scoreboard-kotlin/blob/2eab20acd9805e4bc31fb5c881207db1c515cccd/src/main/kotlin/controller/MatchScoreController.kt#L63C1-L73C6

private fun addRequestAttributes(
    request: HttpServletRequest,
    matchUuid: String,
    gameState: GameState,
) = request.apply {
    val matchScore = OngoingMatchesService.getMatch(matchUuid)
    val players = matchScoreView.getNamesList(matchScore)
    val scoreData = matchScoreView.getMatchScoreDataForView(matchScore, gameState)
    val columnNames = matchScoreView.getColumnNames(gameState)

    setAttribute("uuid", matchUuid)
    setAttribute("firstPlayer", players.first)
    setAttribute("secondPlayer", players.second)
    setAttribute("scoreData", scoreData)
    setAttribute("columnNames", columnNames)
}

Ниже то что я использовал, советую посмотреть этот синтаксис + посмотреть по код стайлу в котлине (можешь обратить внимание на то как в примере аргументы указаны и что нет фигурных скобок метода + будет круто если попробуешь какой-нибудь линтер) + не забывай что отступы в коде могут помочь увидеть какой-то переход в коде https://kotlinlang.ru/docs/scope-functions.html https://kotlinlang.org/api/core/kotlin-stdlib/kotlin/-pair/

6. Для закрытия ресурсов в котлин есть use https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.io/use.html

В дао слое есть дуюлирование execute и executeInTransaction методов, логичней было бы вынести в родительский класс

7. Свои эксепшены советую имплементить от runtime exception (общая практика) + сразу все конструкторы генерировать (которые есть у остальных эксепшенов) чтобы класс в будующем меньше менялся в гите

В целом, как и сказал впечатление спорное, так как с одной стороны цель достигнута - котлин все дела, но с другой проект интересным станет только после рефакторинга. Все ссылки и к какому примерно ввиду всё это вести я приложил)