Рефакторинг контрольного параметра

Question

Я использую инструмент для поиска запахов кода в коде, называемом reek, и у меня есть проблема с одним из них называется Control Parameter

def place_ship(ship, start_position, orientation)
    @row = start_position[:row]
    @column = start_position[:column]
    ship.length.times do
        if orientation == :vertical
            vertical_place_ship(row,column,ship)
        else
            horizontal_place_ship(row,column,ship)
        end
    end
end

def vertical_place_ship(row,column,ship)
    self.grid[row][column].ship = ship
    self.grid[row][column].status = :occupied
    @row += 1 
end

def horizontal_place_ship(row,column,ship)
    self.grid[row][column].ship = ship
    self.grid[row][column].status = :occupied
    @column += 1
end

содержание предупреждения: [55]:ControlParameter: Board#place_ship управляется ориентацией аргумента

Как лучше всего устранить этот запах?

Заранее спасибо,

Мартин

Answer 1

"Ориентация" - это значение флага в методе place_ship. Значение ориентации не меняется при выполнении кода. Так что нет необходимости проверять его время доставки.

У place_ship есть только условная логика и больше ничего. Это не нужно, и условная логика может находиться снаружи. Вы передаете флаг, который указывает методу, какой путь выбрать условно. Это запах условной муфты. В общем случае не передавайте в метод условный параметр. У 2 разных методов для 2 вариантов и назовите их точно.

У вас уже есть метко названные методы vertical_place_ship и Horizontal_place_ship. Вы можете рефакторинг это так.

def <method_that_calls_place_ship>
// other code
    if orientation == :vertical
      vertical_place_ship(ship, start_position)
    else
      horizontal_place_ship(ship, start_position)
    end
// more code
end

def vertical_place_ship(ship, start_position)
    row = start_position[:row]
    column = start_position[:column]

    ship.length.times do
      self.grid[row][column].ship = ship
      self.grid[row][column].status = :occupied
      row += 1 
    end  
end

Аналогично для метода Horizontal_place_ship.

Answer 2

Независимо от обратной связи с инструментом, глядя на ваш код, единственная строка, которая различается между горизонтальным и вертикальным регистром, - это увеличение @rows или @columns. Вариант может быть:

def place_ship(ship, start_position, orientation)
    row = start_position[:row]
    column = start_position[:column]
    ship.length.times do
        self.grid[row][column].ship = ship
        self.grid[row][column].status = :occupied
        orientation == :vertical ? row += 1 : column += 1
    end
end

Я удалил два (идентичных) метода и просто использовал троичный оператор ('?'), Чтобы увеличить правильную переменную после размещения каждой части корабля.