Легко ли следовать этому шаблону попыток извлечения сущностей с шахматной доски?

В настоящее время я пишу партию в шахматы, и, более конкретно, я создаю код для игровой доски. Доска – это просто статический класс, который содержит многомерный массив, представляющий доску, и методы для взаимодействия с ней. Рассматриваемый метод для целей это вопрос, называется TryGetEntity:

private static GameEntity[,] _board = new GameEntity[8,8];
...
public static bool TryGetEntity(BoardCoordinate requestedPosition, out GameEntity occupyingEntity) {
    occupyingEntity = null;
    if (_board.GetLength(0) < requestedPosition.X || _board.GetLength(1) < requestedPosition.Y)
        return false;
    
    if (_board[requestedPosition.X, requestedPosition.Y] != null)
        occupyingEntity = _board[requestedPosition.X, requestedPosition.Y];
    else return false;
    return true;
}

Меня беспокоит эта реализация – возвращаемое значение по умолчанию true. На протяжении многих лет меня учили допускать неудачи, что в общих чертах означает возвращение false по умолчанию. В настоящее время я веду войну с самим собой по поводу рефакторинга этого, чтобы:

occupyingEntity = null;
if (_board.GetLength(0) >= requestedPosition.X || _board.GetLength(1) >= requestedPosition.Y) {
    if (_board[requestedPosition.X, requestedPosition.Y] != null) {
        occupyingEntity = _board[requestedPosition.X, requestedPosition.Y];
        return true;
    }
}

return false;

Я лично считаю, что вторая версия читается лучше, но мне нужно второе (или более) мнение о ней.


Редактировать: По запросу из комментариев, вот весь GameBoard class вместе с примером варианта использования (этот проект все еще находится на очень ранних этапах, поэтому фактических вызовов еще не существует):

public static class GameBoard {
    private static GameEntity[,] _board = new GameEntity[8,8];
    public static bool TryGetEntity(BoardCoordinate requestedPosition, out GameEntity occupyingEntity) {
        occupyingEntity = null;
        if (_board.GetLength(0) >= requestedPosition.X || _board.GetLength(1) >= requestedPosition.Y) {
            if (_board[requestedPosition.X, requestedPosition.Y] != null) {
                occupyingEntity = _board[requestedPosition.X, requestedPosition.Y];
                return true;
            }
        }

        return false;
    }
}

// Example use-case:
var requestedCoordinate = new BoardCoordinate(3, 3);
if (GameBoard.TryGetEntity(requestedCoordinate, out GameEntity occupyingPiece))
    CapturePiece(occupyingPiece);
else
    MoveToPosition(requestedCoordinate);

Кажется ли, что за этой реализацией шаблона try в его первоначально представленном состоянии легко следовать? Как насчет второй версии?

Редактировать: Я считать, что, разместив этот вопрос, я как бы ответил на него. Учитывая, что это заставило меня остановиться и поискать второе мнение в его первоначальном состоянии, я подозреваю, что то же самое можно сказать и о других. Кроме того, теперь, когда я немного поспал, первую версию читать намного сложнее, чем вторую. Это не более сложно, просто, загромождено и слишком много. Я сохраню этот вопрос, чтобы другие ответили более подробно, но я не удивлюсь, если все согласятся, что «первый метод труднее читать».

Примечание: Давайте проигнорируем форматирование этого сообщения.

6 ответов
6

Первая версия:

  • Не следует опускать фигурные скобки {} хотя они могут быть необязательными, потому что это может привести к скрытым и трудно отслеживаемым ошибкам. Наличие фигурных скобок также помогает структурировать код.
  • Вместо else ты просто мог вернуться пораньше в if блокировать и иметь return false; после.

Это приведет к

public static bool TryGetEntity(BoardCoordinate requestedPosition, out GameEntity occupyingEntity) 
{
    occupyingEntity = null;
    if (_board.GetLength(0) < requestedPosition.X || _board.GetLength(1) < requestedPosition.Y)
    {
        return false;
    }

    if (_board[requestedPosition.X, requestedPosition.Y] != null)
    {
        occupyingEntity = _board[requestedPosition.X, requestedPosition.Y];
        return true;
    }
    return false;
}  

который ИМО легко читать.

Вторая версия:

Ну думаю вложено if's ухудшает читаемость. Я бы хотя бы сохранил первый if отдельно и поменял бы ….. нет, забудь. Я бы просто взял первую версию и поменял вот так

public static bool TryGetEntity(BoardCoordinate requestedPosition, out GameEntity occupyingEntity) 
{
    occupyingEntity = null;
    if (_board.GetLength(0) < requestedPosition.X || _board.GetLength(1) < requestedPosition.Y)
    {
        return false;
    }

    occupyingEntity = _board[requestedPosition.X, requestedPosition.Y];
    return occupyingEntity != null ;
}

  • 1

    Просто потому что ты может повсюду ставить фигурные скобки, это не значит, что ты иметь. Когда у тебя много if... else if ... else if ... else и все они однострочны, вы удваиваете длину, добавляя лишние скобки. Я согласен, однако, что если хотя бы один из ifнужны подтяжки, весь блок должен их получить.

    – Хаукингер

Да, код читабельный.

Мне любопытно, почему вы считаете, что вам нужно несколько операторов возврата. Оператор return может возвращать логическое выражение.

public static class GameBoard
{
    private static GameEntity[,] _board = new GameEntity[8, 8];
    public static bool TryGetEntity(BoardCoordinate requestedPosition, out GameEntity occupyingEntity)
    {
        occupyingEntity = null;
        if (_board.GetLength(0) >= requestedPosition.X || _board.GetLength(1) >= requestedPosition.Y)
        {
            if (_board[requestedPosition.X, requestedPosition.Y] != null)
            {
                occupyingEntity = _board[requestedPosition.X, requestedPosition.Y];
            }
        }

        return occupyingEntity != null;
    }
}

  • 1

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

    – Тако Тако

  • 3

    ИМХО это плохое решение. _board[requestedPosition.X, requestedPosition.Y] используется дважды, плюс есть второй уровень if и этот второй уровень даже не нужен, так как если _board[requestedPosition.X, requestedPosition.Y] имеет значение null, нет опасности присвоить это значение параметру occupyingEntity в любом случае.

    – BCdotWEB

  • @BCdotWEB IMHO, этот ответ верен сообщению, которое он пытается передать, независимо от доступа к избыточному массиву. Вывод здесь – вернуть выражение вместо использования нескольких операторов возврата. Таким образом, я бы не сказал, что это плохое решение, на самом деле это отличное решение; он просто контролирует избыточность.

    – Тако Тако

Я не знаю C #, но для этого нужны типы Nullable. Насколько я могу судить, они устарели TryX+out комбинация параметров.

Итак, ваша подпись метода должна быть больше похожа на:

public static GameEntity? GetEntity(BoardCoordinate requestedPosition)

Если вы либо напрямую возвращаете действительный GameEntity, либо просто return null.

Не уверен, что это был ответ, о котором вы говорили ранее, но это замечательный момент. Поскольку вы указали, что плохо разбираетесь в C #, я хотел бы отметить, что GameEntity является ссылочным типом и по умолчанию имеет значение null. Однако было бы неплохо, если бы я добавил явное объявление, что null разрешено в моем методе с помощью аннотации, допускающей значение NULL GameEntity? как вы предлагаете. +1

– Тако Тако


Ваша первая версия TryGetEntity по существу следует этой структуре:

public static bool TryGetEntity(BoardCoordinate x, out GameEntity ret) {
    ret = null;
    if (condition1)
        return false;
    
    if (! condition2)
        ret = value;
    else return false;
    return true;
}

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

public static bool TryGetEntity(BoardCoordinate x, out GameEntity ret) {
    ret = null;

    if (condition1)
        return false;
    if (condition2)
        return false;
    
    ret = value;
    return true;
}

Это делает логику более очевидной.

    Для меня код, представленный @pacmaninbw, близок к тому, что я написал бы, но я бы упростил его, чтобы избежать второго оператора if:

    public static bool TryGetEntity(BoardCoordinate requestedPosition, out GameEntity occupyingEntity) {
         occupyingEntity = null;
    
         if (_board.GetLength(0) >= requestedPosition.X || _board.GetLength(1) >= requestedPosition.Y) {
              occupyingEntity = _board[requestedPosition.X, requestedPosition.Y];
         }
    
         return occupyingEntity != null;
    }
    

    Я также считаю, что этот стиль {...} менее эффективен, что упрощает его последовательное добавление повсюду.

      Добавить комментарий

      Ваш адрес email не будет опубликован. Обязательные поля помечены *