Ruby: Refactoring a complicated nested-loop method

Question

I'm trying to get rid of duplication in my code. I have a method that populates a checkerboard with checkers:

def populate_checkers
  evens = [0, 2, 4, 6]
  odds  = [1, 3, 5, 7]

  0.upto(2) do |x_coord|
    if x_coord.even?
      evens.each do |y_coord|
        red_checker = Checker.new(x_coord, y_coord, :red)
        @board[x_coord][y_coord] = red_checker 
      end
    elsif x_coord.odd?
      odds.each do |y_coord|
        red_checker = Checker.new(x_coord, y_coord, :red)
        @board[x_coord][y_coord] = red_checker 
      end
    end
  end

  5.upto(7) do |x_coord|
    if x_coord.even?
      evens.each do |y_coord|
        black_checker = Checker.new(x_coord, y_coord, :black)
        @board[x_coord][y_coord] = black_checker 
      end
    elsif x_coord.odd?
      odds.each do |y_coord|
        black_checker = Checker.new(x_coord, y_coord, :black)
        @board[x_coord][y_coord] = black_checker 
      end
    end
  end
end

How can I remove duplication and still get the precise behavior I need?

side note: from a functional-programming perspective this kind of code is very, very bad. You call a method and "magically" some instance variable (@board) is filled, alas, referential transparency going down the hill. Better write methods that get arguments and return something: @board = build_board. More on FP with Ruby: slideshare.net/tokland/functional-programming-with-ruby-9975242 — tokland
– tokland, Commented Dec 7, 2011 at 13:54

Aliaksei Kliuchnikau · Accepted Answer · 2011-12-07 12:48:54Z

3

You can try to extract a method and then extract a block into lambda. Then your code will be readable and loose of duplication

def populate_checkers
  0.upto(2) do |x_coord|
    populate_checker(x_coord, :red)
  end

  5.upto(7) do |x_coord|
    populate_checker(x_cord, :black)
  end
end

def populate_checker(x_coord, color)
  evens = [0, 2, 4, 6]
  odds  = [1, 3, 5, 7]

  apply_checker = lambda do |y_coord|
   checker = Checker.new(x_coord, y_coord, color)
   @board[x_coord][y_coord] = checker
  end

  if x_coord.even?
    evens.each(&apply_checker)
  elsif x_coord.odd?
    odds.each(&apply_checker)
  end
end

answered Dec 7, 2011 at 12:48

Aliaksei Kliuchnikau

13.7k4 gold badges64 silver badges74 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

andrew12 · Accepted Answer · 2011-12-07 13:08:52Z

2

def populate_checkers
  evens = [0, 2, 4, 6]
  odds  = [1, 3, 5, 7]

  [0.upto(2), 5.upto(7)].each_with_index do |enum, i|
    enum.each do |x_coord|
      (x_coord.even? ? evens : odds).each do |y_coord|
        checker = Checker.new(x_coord, y_coord, i == 0 ? :red : :black)
        @board[x_coord][y_coord] = checker 
      end
    end
  end
end

There's probably a better way to do the counting bit, but that's what I got.

Here's a possibly better solution...

def populate_checkers
  { :red => (0..2), :black => (5..7) }.each do |color, range|
    range.each do |x_coord|
      (x_coord.even? ? 0 : 1).step(7, 2) do |y_coord|
        checker = Checker.new(x_coord, y_coord, color)
        @board[x_coord][y_coord] = checker 
      end
    end
  end
end

edited Dec 7, 2011 at 13:08

answered Dec 7, 2011 at 12:43

andrew12

313 bronze badges

Comments

steenslag · Accepted Answer · 2011-12-07 13:02:33Z

1

0.upto(2) do |x|
  0.upto(7) do |y|
    @board[x][y]=Checker.new(x, y, :red) if (x+y).even? 
  end
end

This is only for the reds.

answered Dec 7, 2011 at 13:02

steenslag

80.2k16 gold badges144 silver badges174 bronze badges

Collectives™ on Stack Overflow

Ruby: Refactoring a complicated nested-loop method

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related