如何修复Reek检测出的Board#place_ship方法Control Parameter代码异味？

That Control Parameter warning from Reek is pointing out a classic code smell: your place_ship method is doing two distinct things depending on the orientation argument, which violates the Single Responsibility Principle. Let’s break down clean, practical ways to fix this.

Option 1: Split the Method (Simplest & Most Direct Fix) #

The easiest way to eliminate the control parameter is to split place_ship into two separate methods, each dedicated to one orientation. This makes each method’s purpose explicit and removes conditional logic entirely.

Here’s the refactored code:

class Board
  # Replace the original place_ship with these two methods
  def place_vertical_ship(ship, start_position)
    row = start_position[:row]
    column = start_position[:column]
    
    ship.length.times do
      grid[row][column].ship = ship
      grid[row][column].status = :occupied
      row += 1
    end
  end

  def place_horizontal_ship(ship, start_position)
    row = start_position[:row]
    column = start_position[:column]
    
    ship.length.times do
      grid[row][column].ship = ship
      grid[row][column].status = :occupied
      column += 1
    end
  end
end

Why this works: #

• Each method now has a single, well-defined job (placing a ship vertically or horizontally).
• No more branching based on a control parameter—Reek’s warning will disappear immediately.
• Callers of your code will be explicit about which placement direction they need, making the code easier to read and debug.

Option 2: Use a Strategy Pattern (For Extensibility) #

If you anticipate adding more placement orientations later (like diagonal, for example), the strategy pattern is a robust choice. It encapsulates each placement logic into its own object, keeping your Board class focused on core responsibilities.

First, define strategy classes for each orientation:

class VerticalPlacementStrategy
  def next_coordinate(row, column)
    [row + 1, column]
  end
end

class HorizontalPlacementStrategy
  def next_coordinate(row, column)
    [row, column + 1]
  end
end

Then refactor place_ship to accept a strategy object instead of an orientation symbol:

class Board
  def place_ship(ship, start_position, placement_strategy)
    row = start_position[:row]
    column = start_position[:column]
    
    ship.length.times do
      grid[row][column].ship = ship
      grid[row][column].status = :occupied
      row, column = placement_strategy.next_coordinate(row, column)
    end
  end
end

How to use it: #

# Place a vertical ship
board.place_ship(
  my_ship,
  { row: 2, column: 3 },
  VerticalPlacementStrategy.new
)

# Place a horizontal ship
board.place_ship(
  my_ship,
  { row: 5, column: 1 },
  HorizontalPlacementStrategy.new
)

Why this works: #

• Your place_ship method now only handles the core logic of placing ship segments, delegating coordinate updates to the strategy.
• Adding a new orientation only requires creating a new strategy class—no changes to the Board class needed (follows the Open/Closed Principle).
• The control parameter is replaced with explicit strategy objects, making the code more flexible and maintainable.

Which Option Should You Choose? #

• Go with Option 1 if your placement logic is simple and you don’t expect to add more orientations anytime soon. It’s quick, clean, and easy to understand.
• Go with Option 2 if you need extensibility or if your placement rules might get more complex over time.

内容的提问来源于stack exchange，提问作者martin123154