2

我正在使用一种工具来查找名为 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

我该如何解决？

4

2 回答 2

6

“方向”是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 方法也是如此。

于 2018-01-07T16:34:01.817 回答
0

无论工具反馈如何，查看您的代码，水平和垂直情况之间的唯一区别在于是否增加@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

我删除了两个（相同的）方法，只是在放置每个船部件后使用三元运算符（'？'）来增加正确的变量。

于 2018-01-07T15:38:20.910 回答