Я пытаюсь написать код для определения выигрыша в игре в крестики-нолики. В этом вопросе я не сосредотачиваюсь на всей игре, меня интересует только вопрос, как определить, что игра закончилась победой одного из игроков. Итак, я сделал синтетический пример для тестирования различных подходов.

Я предполагаю, что выигрышная линия на доске может появиться только после того, как текущий игрок сделает свой ход. Поэтому нет смысла проверять все линии на доске. Я считаю только те, которые включают последний сделанный ход. Я предполагаю, что доска может быть любого размера (но только квадратная), а выигрышная линия такого же размера. Таким образом, это одна из строк, или одна из колонок, или одна из двух диагоналей.

После нескольких попыток я получил следующий код:

import java.util.List;
import java.util.function.Predicate;

enum Cell {
    X,
    O,
    B // blank cell
}

public class Board {

    private final int size = 3;
    private final Cell[][] board;

    public Board(Cell[][] board) {
        this.board = board;
    }

    private boolean isLineCompleted(Predicate<Integer> predicate) {
        boolean result = true;
        int i = 0;
        while (result && i < size) {
            result = predicate.test(i);
            i++;
        }
        return result;
    }

    public boolean hasWinningLine(int row, int col) {
        if (board[row][col] == Cell.B) return false;

        Cell base = board[row][col];
        List<Predicate<Integer>> checks = List.of(
                i -> board[row][i] == base, //columns
                i -> board[i][col] == base, //rows
                i -> board[i][i] == base,   //top-down diagonal
                i -> board[size - i - 1][i] == base //down-top diagonal
        );

        boolean lineCompleted = false;
        for (var check : checks) {
            if (!lineCompleted) {
                lineCompleted = isLineCompleted(check);
            }
        }

        return lineCompleted;
    }
}

И я написал несколько тестов, которые используют этот код примерно так:

Cell[][] xWins = {
    {Cell.X, Cell.O, Cell.X},
    {Cell.O, Cell.X, Cell.O},
    {Cell.X, Cell.B, Cell.B}};

board = new Board(xWins);
AssertTrue(board.hasWinningLine(0, 2));

У меня несколько вопросов:

1. Насколько легко читать и понимать этот код? Как вы можете улучшить его в этом смысле?

2. Стараюсь не делать лишних проверок. Алгоритм должен вернуть false, как только найдет неподходящую ячейку. Должен ли я обрабатывать петлю вот так

for (var check : checks) {
            if (!lineCompleted) {
                lineCompleted = isLineCompleted(check);
            } else break; // That's better? 
        }

Можно ли переписать этот цикл с помощью потокового API?

3. При проверке каждой строки этот код может сравнивать ячейку с собой. Как этого избежать, не усложняя код? Стоит ли мне даже думать об этом?

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

EDITED: цикл проверки предикатов можно переписать как

return checks.stream()
    .map(this::isLineCompleted)
    .filter(b -> b == true)
    .findFirst()
    .orElse(false);

В чем недостатки такого подхода?

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

Как правило, ваш код может быть слишком сложным / многословным. Например, isLineCompleted было бы намного проще использовать обычный for цикл и ранний выход:

private boolean isLineCompleted(Predicate<Integer> predicate) {
    for (int i = 0; i < size; i++) {
         if (predicate.test(i)) {
             return true;
         }
    }
    return false;
}

Путем извлечения checks цикл в отдельный метод, там можно было бы использовать тот же шаблон.

Или Java Stream имеет .anyMatch() метод, который еще больше его упрощает:

return checks.stream().anyMatch(this::isLineCompleted);

Использование IntStream позволяет isLineCompleted делать симуляцию с использованием .allMatch():

private boolean isLineCompleted(Predicate<Integer> predicate) {
    return IntStream.range(0, size).allMatch(predicate);
}

И последний момент: ценность B в Cell enum плохо назван, учитывая, что вам нужно добавить комментарий «пустая ячейка». Назови это BLANK вместо. Также логичнее было бы поместить его первым в перечислении, где он будет иметь кардинальное значение 0. Или вы можете даже отказаться от всего этого и просто использовать null.

Ошибки

Размер доски

Предполагаю, что доска может быть любого размера (но только квадратная) …

private final int size = 3;

Вы имеете в виду, что доска может быть любого размера, если она 3 на 3?

Возможно, вы имели в виду …

    private final int size;
    private final Cell[][] board;

    public Board(Cell[][] board) {
        this.board = board;
        this.size = board.length;
    }

hasWinningLine (строка, столбец)

Я считаю только те, которые включают последний сделанный ход.

В вашем тестовом примере board.hasWinningLine(0, 0) вернется true несмотря на 0, 0 не быть на выигрышной линии. Ваш код безоговорочно проверяет диагонали.

Вместо этого ваш код должен только добавлять диагональные проверки, если последний ход был на этих диагоналях:

        List<Predicate<Integer>> checks = new ArrayList<>();
        checks.add(i -> board[row][i] == base);
        checks.add(i -> board[i][col] == base);
        if (row == col)
            checks.add(i -> board[i][i] == base);
        if (size - row - 1 == col)
            checks.add(i -> board[size - i - 1][i] == base);

        ...

Вместо создания временного списка мы могли бы напрямую создать поток предикатов. Например, используя .anyMatch() как предлагается в ответе RoToRa:

        Stream.Builder<Predicate<Integer>> builder = Stream.builder();
        builder.add(i -> board[row][i] == base);
        builder.add(i -> board[i][col] == base);
        if (row == col)
            builder.add(i -> board[i][i] == base);
        if (size - row - 1 == col)
            builder.add(i -> board[size - i - 1][i] == base);

        Stream<Predicate<Integer>> stream = builder.build();
        return stream.anyMatch(this::isLineCompleted);