Может ли метод быть читаемым и короче?

Question

Я написал этот метод:

private int maxSequence (char player , Cell c)
{
    int row = c.getRow();
    int col = c.getCol();
    int maxVert = 0;
    int maxHor = 0;
    int maxDiag = 0;

    if (player == 'O')
    { 

        for (int j = 0; j < _board[0].length; j++)
        {
            if ( (_board[col][row+j] == 'O') || (_board[col][row-j] == 'O') )
            {
                maxVert++;
            }

            if ( (_board[col+j][row] == 'O') || (_board[col-j][row] == 'O') )
            {
                maxHor++;
            }

            if ( (_board[col+j][row+j] == 'O') || (_board[col-j][row-j] == 'O') )
            {
                maxDiag++;
            }
        }
    }

    if (player == 'X')
    {
        for (int j = 0; j < _board[0].length; j++)
        {
            if ( (_board[col][row+j] == 'O') || (_board[col][row-j] == 'X') )
            {
                maxVert++;
            }

            if ( (_board[col+j][row] == 'O') || (_board[col-j][row] == 'X') )
            {
                maxHor++;
            }

            if ( (_board[col+j][row+j] == 'O') || (_board[col-j][row-j] == 'X') )
            {
                maxDiag++;
            }
        }
    }

    if ( (maxDiag >= maxVert) && (maxDiag >= maxHor) )
    {
        return maxDiag;
    }

    else if ( (maxVert >= maxDiag) && (maxVert >= maxHor) )
    {
        return maxVert;
    }

    else
    {
        return maxHor;
    }
}

Интересно, есть ли способ улучшить метод, чтобы он был читабельным и \ или более коротким?

Answer 1

Использование O в качестве имени переменной - плохая идея. O может быть неправильно прочитан или опечатан как 0 или наоборот, , как вы, кажется, сделали в нескольких местах.

В этом случае, я думаю, вы должны использовать что-то вроде O_PLAYER и O_MARKER вместо O по вышеуказанной причине, а также чтобы различать два различных значения константы в вашем коде. (На самом деле, в зависимости от более широкого контекста, я мог бы создать пару типов enum для этих двух случаев.)

Размещение открывающих фигурных скобок на предыдущей строке экономит место и является (IMO) таким же читабельным; например,

if (cond) {
   // blah
} else {
   // blah
}

против

if (cond) 
{
   // blah
} 
else 
{
   // blah
}

Префикс переменных экземпляра с _ нарушает стандарт именования Java.

Наконец, последние 12 строк можно переписать так:

return Math.max(maxHor, Math.max(maxVert, maxDiag));

Answer 2

Не полный ответ здесь, извините, пить кофе и лениться этим утром (также этот вид выглядит как школьная проблема).Поэтому я буду держать его на высоком уровне.

1. Не бойтесь методов, они могут значительно улучшить ясность.Ваши циклы for могут быть заключены в метод, в котором имя объясняет, что происходит.
2. Избегайте использования магических значений, замените символы 'X' на константу.
3. Еще лучше оберните символ классом игрока.Player.for ('x') возвращает класс игрока.Делегируйте поведение в ваших циклах классам XPlayer, YPlayer.
4. Как и в других постерах, где это возможно, используйте Math.max и другие встроенные функции

Answer 3

Построить сложные однострочники в большинстве случаев плохая идея.Однако - в некоторых случаях, например, здесь, вы облегчаете зрение, что является общим для нескольких рядов, а что отличается:

private int maxSequence (char player , Cell c)
    {
        int row = c.getRow();
        int col = c.getCol();
        int maxVert = 0;
        int maxHor = 0;
        int maxDiag = 0;

        if (player == O)
        {
            for (int j = 0; j < _board[0].length; j++)
            {
                if ((_board [col]  [row+j] == O) || (_board [col]  [row-j] == O))  maxVert++;
                if ((_board [col+j][row]   == O) || (_board [col-j][row]   == O))  maxHor++;
                if ((_board [col+j][row+j] == 0) || (_board [col-j][row-j] == O))  maxDiag++;
            }
        }
        if (player == X)
        {
            for (int j = 0; j < _board[0].length; j++)
            {
                if ((_board [col]  [row+j] == O) || (_board [col]  [row-j] == X))   maxVert++;
                if ((_board [col+j][row]   == O) || (_board [col-j][row]   == X))   maxHor++;
                if ((_board [col+j][row+j] == 0) || (_board [col-j][row-j] == X))   maxDiag++;
            }
        }
        if ((maxDiag >= maxVert) && (maxDiag >= maxHor))    return maxDiag;
        if ((maxVert >= maxDiag) && (maxVert >= maxHor))    return maxVert;
        return maxHor;
    }

После обращения к строкам я заметил, что в первомблок, вы тестируете на OO, OO, OO, а во втором блоке на OX, OX, OX, и я сомневаюсь, вы хотели бы проверить на XX, XX, XX.

Без компиляции, во втором представлении я вижу, что есть OO0, где я бы ожидал OOO (не так ли было XXX, как упомянуто выше).:) - Ах, но этот шаблон используется и в чистом O-блоке.

Я хотел бы добавить, что мне нравится идея Stehpen C выразить max (a, b, c), что первые два блока должны быть заменены параметризованным блоком, и что более полная картина всей программыможет привести к совершенно другому дизайну, более объектно-ориентированному.

Объединяя предложения, мы промежуточно останавливаемся здесь:

private int maxSequence (char player, Cell c)
{
    int row = c.getRow();
    int col = c.getCol();
    int maxVert = 0;
    int maxHor = 0;
    int maxDiag = 0;

    for (int j = 0; j < _board[0].length; j++)
    {
        if ((board [col]  [row+j] == player) || (board [col]  [row-j] == player))  maxVert++;
        if ((board [col+j][row]   == player) || (board [col-j][row]   == player))  maxHor++;
        if ((board [col+j][row+j] == player) || (board [col-j][row-j] == player))  maxDiag++;
    }
    return Math.max (maxHor, Math.max (maxVert, maxDiag));
}

Промежуточно, потому что мы могли бы сделать совершенно другой дизайн - я чувствую запах класса Board и класса Player по крайней мере,

Answer 4

private int maxSequence (char player , Cell c)
{
    int row = c.getRow();
    int col = c.getCol();
    int maxVert = 0;
    int maxHor = 0;
    int maxDiag = 0;

    for (int j = 0; j < _board[0].length; j++)
    {
        if ( (_board[col][row+j] == O) || (_board[col][row-j] == player) )
        {
            maxVert++;
        }

        if ( (_board[col+j][row] == O) || (_board[col-j][row] == player) )
        {
            maxHor++;
        }

        if ( (_board[col+j][row+j] == 0) || (_board[col-j][row-j] == player) )
        {
            maxDiag++;
        }
    }

    return Math.max(maxDiag, Math.max(maxVert, maxHor));
}

Answer 5

Помимо того факта, что это не скомпилируется (если X и O не являются переменными экземпляра) ...

Чем код для игрока 'O' отличается от кода игрока 'X'?Ответ - второе выражение в каждом операторе if ... замените его переменной, и вы урежете код пополам.

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

Answer 6

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

Также вы можете объединить циклы for в один, так как они выполняют аналогичную работу

if (board[col][row + j] == O || board[col][row - j] == O) 


        if (maxDiag >= maxVert && maxDiag >= maxHor) {
            return maxDiag;
        }else if (maxVert >= maxDiag && maxVert >= maxHor) {
            return maxVert;
        }else {
            return maxHor;
        }