ruby - 重构控制参数

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 is controlled by argument 'orientation

我该如何解决这个问题？

Answer 1

'Orientation' 是place_ship 方法中的标志值。 'orientation' 的值不会随着代码的执行而改变。因此无需检查它 'ship.length' 次。

place_ship 只有条件逻辑，没有别的。这是不必要的，条件逻辑可以驻留在外部。您正在传递一个标志，告诉方法有条件地选择什么路径。这就是条件耦合的味道。通常不要将条件参数传递给方法。为 2 个选择提供 2 种不同的方法，并恰本地命名它们。

您已经恰本地命名了 vertical_place_ship 和 horizo​​ntal_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

与 horizo​​ntal_place_ship 方法类似。