Advanced validation

[loskutovanl]

Сделала валидацию advanced - сервис №2 из validation.md

Написала тесты ко всем функциям, добавила юнит тесты и Fuzz тесты. go test -fuzz FuzzAdvanced . на время еще не запускала.

Налетайте...

Пока делала, поняла, что мы нигде не валидируем саму доску (например, доска с 2 королями, доска без королей и т.п.). Возник вопрос нужно ли это где-то делать вообще?

[sadmadrus]

Доску, по-идее, гененрирует сервис игровой сессии. А вот нужна валидация доски потом - нужно подумать

[nekr0z]

Насчёт валидации доски — это надо написать; я могу заняться.

[sadmadrus]

Насчёт валидации доски — это надо написать; я могу заняться.

Если никто не против, то давай

[nekr0z]

Насчёт валидации доски — это надо написать; я могу заняться.

Если никто не против, то давай

34 — сделаю, рассчитываю до конца недели, если без форс-мажоров.

[loskutovanl]

В общем и целом LGTM; несколько замечаний написал :)

Спасибо за обратную связь, хоть кто-то всё прочитал 😄

Буду править потихоньку, по наличию свободного времени, отпишусь когда всё

[loskutovanl]

Внесла исправления в соответствии с замечаниями

[loskutovanl]

Удалила свою логику по определению шахов, использовала логику @nekr0z из position.ThreatsTo. Столкнулась с проблемой при валидации рокировки. Когда проверяю, проходит ли король через битое поле - я проверяю на шах клетку, в которой нет фигуры.

Чтобы не плодить лишние сотни строк моих проверок, Предлагаю добавить обработку этого непосредственно в position.ThreatsTo, например в таком виде:

func ThreatsTo(s board.Square, b board.Board, isW bool) []board.Square {
    if s < 0 || s > 63 {
        return nil
    }
    var isW, isB bool
    var out []board.Square
    if getPiece(b, s) != 0 {
        if getPiece(b, s)%2 == 0 {
            isB = true
                        isW = false
        } else {
            isW = true
        }
    } else {
                if !isW {
                         isB = true
                }
        }

[nekr0z]

Чтобы не плодить лишние сотни строк моих проверок, Предлагаю добавить обработку этого непосредственно в position.ThreatsTo

У меня была эта мысль, но я решил, что проще будет у тебя в validation набросать что-то вроде:

func squareUnderAttack(b board.Board, s board.Square, weAreWhite bool) bool {
    p := board.BlackPawn
    if weAreWhite {
        p = board.WhitePawn
    }
    err := b.Put(s, p)
    if err != nil {
        // кто-то пихает невалидный Square, да и хрен с ним
        return false
    }
    return len(position.ThreatsTo(s, b)) > 0
}

;-)

[nekr0z]

    if err != nil {
        // кто-то пихает невалидный Square, да и хрен с ним
        return false
    }

Это в реальности немножно не работает, потому что Put() выдаёт ошибку и в том случае, если клетка уже занята ;) Но в этом случае можно просто проверять об имеющуюся фигуру.

Сейчас будет PR :)

[nekr0z]

@loskutovanl #40

[loskutovanl]

ну вроде хорошо получилось :) Надо что-то еще править?

КМК, Вариант 1. Сливаем и радуемся. Вариант 2. Я могу переделать все функции так, чтобы они работали с типом board.Square вместе локального square. Но тогда я захочу расширять методы board.Square для своего удобства diffRow, diffColumn и т.п. наподобие того что у меня написано вот здесь. Не странно будет, если такое приедет в пакет board?

[nekr0z]

Не странно будет, если такое приедет в пакет board?

Вопрос не в странности, вопрос в полезности.

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

В Go расширять импортированные типы новыми методами нельзя, и писать «обёртки» — вроде твоего square — нормальная практика. Мне нравится подход, при котором если конкретная обёртка нужна в одном пакете, она остаётся обёрткой, а если в двух-трёх пакетах оказываются востребованы одинаковые обёртки, нужные методы внедряются в исходный пакет (или пишется отдельный пакет с обёрткой).

[loskutovanl]

В Go расширять импортированные типы новыми методами нельзя, и писать «обёртки» — вроде твоего square — нормальная практика. Мне нравится подход, при котором если конкретная обёртка нужна в одном пакете, она остаётся обёрткой, а если в двух-трёх пакетах оказываются востребованы одинаковые обёртки, нужные методы внедряются в исходный пакет (или пишется отдельный пакет с обёрткой).

В таком случае оставляем как есть

[loskutovanl]

смержила изменения dev сюда.

Мержим в дев?