разжалование валидации из микросервисов

[loskutovanl]

Насколько я понимаю задачу, требуется сделать следующее. Поправьте меня, если нужно.

TODO:

• [x] Пересмотреть advancedLogic (нет смысла функции advancedLogic возвращать новую доску и параметр ok.)
• [x] нет смысла в контроллерах http (удалить?)
• [x] Нет смысла в parse функциях (удалить?)
• [x] Нет смысла в validation.md (удалить?)
• [x] Перенос из internal в отдельный пакет, к board или свой.
• [x] Функция, которая возвращает []board.Square, куда можно сходить из заданной board.Square, потенциально будет нужна (экспортируемый аналог getAvailableMoves).
• [x] нужна единая логика возвращения ошибок из функций
• [x] баг с тестами GetAvailableMoves - пофиксить

[sadmadrus]

а md зачем удалять? описание пакета все равно нужно же

[loskutovanl]

а md зачем удалять? описание пакета все равно нужно же

Просто фактически это преобразуется в две экспортируемые функции-проверка валидности хода, и получение всех возможных ходов. Для этого нужна документация?

Я имею в виду, что мы вроде не писали подробную доку на сам пакет board.

[sadmadrus]

а md зачем удалять? описание пакета все равно нужно же

Просто фактически это преобразуется в две экспортируемые функции-проверка валидности хода, и получение всех возможных ходов. Для этого нужна документация?

Я имею в виду, что мы вроде не писали подробную доку на сам пакет board.

документации много не бывает :)

[nekr0z]

документации много не бывает :)

Ещё как бывает!

В Go не просто так принято писать документацию к пакетам комментами к коду (и читать её потом через godoc): так меньше вероятность, что, по мере работы над кодом, документация и реальное положение дел сильно разойдутся — и даже в Go это в итоге случается, сам видел. Документация же во внешнем файле — это вечная боль, потому что надо же не забывать её обновлять, когда что-то меняется.

Поэтому отдельная дока для микросервиса — это нормально: микросервис, считай — отдельная программа, и отдельная readme'ха к программе, в которой описаны функционал и правила пользования, не затрагивая детали имплементации — это логично и привычно (и пользователю — в случае микросервиса это пользователь API, то есть другой разраб, — и самому разрабу сервиса), а вот отдельная дока к пакету — это не путь Go, а создание на ровном месте головняка в дальнейшей поддержке.

[loskutovanl]

Готово для ревью. Постаралась упростить всё как можно больше и четко разделить, где валидация хода, а где проверка входных данных (возможно, у вас другое видение какие входные данные считаются невалидными). Тесты починила. Задокументировала комментариями к функциям, всё лишнее удалила.

[loskutovanl]

fuzz добавила, нужен совет.

Сейчас у меня стоит вот такой костыль:

f.Fuzz(func(t *testing.T, brd string, from int) {
        b, _ := board.FromFEN(brd)
        if b != nil { // TODO: убрать это (добавить в валидацию доски проверку на nil?)
            res, err := GetAvailableMoves(*b, board.Sq(from))
...
}

Кажется где-то я видела, что добавлять проверку на nil при обработке это хороший тон. Имеет смысл в position.IsValid сделать что-то такое? Или как идиоматично?

func IsValid(b board.Board) bool {
    if b == nil {
        return false
    }
    bKing := board.Square(-1)
    wKing := board.Square(-1)
...
}

[nekr0z]

Имеет смысл в position.IsValid сделать что-то такое?

А ты попробуй ;)

[sadmadrus]

документации много не бывает :)

Ещё как бывает!

В Go не просто так принято писать документацию к пакетам комментами к коду (и читать её потом через godoc): так меньше вероятность, что, по мере работы над кодом, документация и реальное положение дел сильно разойдутся — и даже в Go это в итоге случается, сам видел. Документация же во внешнем файле — это вечная боль, потому что надо же не забывать её обновлять, когда что-то меняется.

Поэтому отдельная дока для микросервиса — это нормально: микросервис, считай — отдельная программа, и отдельная readme'ха к программе, в которой описаны функционал и правила пользования, не затрагивая детали имплементации — это логично и привычно (и пользователю — в случае микросервиса это пользователь API, то есть другой разраб, — и самому разрабу сервиса), а вот отдельная дока к пакету — это не путь Go, а создание на ровном месте головняка в дальнейшей поддержке.

каюсь - привычки крепки. Автогенерация документации для меня новинка :)

[loskutovanl]

А ты попробуй ;)

окей, я попробовала, не работает. насколько я поняла, единственный способ избежать паники, всегда вручную проверять что указатель на доску не nil

[nekr0z]

окей, я попробовала, не работает. насколько я поняла, единственный способ избежать паники, всегда вручную проверять что указатель на доску не nil

Да.

Фишка в том, что IsValid() принимает не указатель, а сам тип board.Board, а он не может быть nil. Больше того, если помнишь, в board нарочно сделано, чтобы неинициализированное значение board.Board было валидной пустой доской.

А вот в тесте, где ты генерируешь FEN фаззером, тебе может вернуться нулевой указатель, и придётся это проверять. Потому что проверяемые функции ожидают board.Board, а не указатель, и указатель тебе приходится разыменовывать, а разыменование nil вызовет, конечно, панику.

[loskutovanl]

тогда открытых вопросов больше нет:)