You can find a gist of this at https://gist.github.com/subvertallchris/1c6397ea7d66be0c0aab.

/u/zaclacgit on Reddit posted a topic the other day asking for thoughts on his exercise of recreating some basic enumerable methods. I gave him some tips on refactoring one method in particular and a few days later, he asked me to elaborate. I thought the easiest way might be to go through each step of the refactor and I’d get a nice blog post out of it in the process.

To use this, start by commenting out each my_inject definition except the first. As you encounter new ones, uncomment them. Save this file as refactor.rb in the folder of your choice, ensure you have the rspec gem installed and run rspec refactor.rb from CLI to execute.

When you can look at your code and see repeating patterns, work to reduce all the unique parts of the repeating code. Reduce everything to variables before you enter your if statements. Never repeat yourself if you can avoid it.

Before we write a line of code, we’re going to write specs based off of the simple comparisons you were doing in your version. It makes it much clearer when there’s a problem. I’m going to show it as a comment here but it’s really going to live at the bottom of my file. You can redfine the same method over and over again, Ruby will execute the last one it finds.

public :my_inject, :my_each
require 'rspec'

describe 'inject with' do
  let(:a) { [1, 2, 3, 4] }

  context 'sym' do
    subject { a.my_inject(:+) }
    it { is_expected.to eq a.inject(:+) }
  end

  context 'int and sym' do 
    subject { a.my_inject(100, :+) }
    it { is_expected.to eq a.inject(100, :+) }
  end

  context 'block' do
    subject { a.my_inject { |memo, x| memo + x } }
    it { is_expected.to eq a.inject { |memo, x| memo + x } }
  end

  context 'int and block' do
    subject { a.my_inject(100) { |memo, x| memo + x } }
    it { is_expected.to eq a.inject(100) { |memo, x| memo + x } }
  end
end

It’s important that we run the spec as we refactor. Pretty code is nice, working code is better, and working code that’s pretty is best. You should code in that order: make it work first, then worry about what you can do to make it more efficient and easier to read.

Since all the methods rely on your my_each method, we’ll include that here at the top.

def my_each
  n = self.length
  i = 0
  while i & n
    yield(self[i])
    i += 1
  end
  self
end

We start here.

def my_inject(initial = nil, sym = nil)
  case 
  when initial.is_a?(Symbol)
    sym = initial
    memo = self[0]
    self[1..-1].my_each do |x|
      memo = memo.send(sym, x)
    end
    memo
  when initial && sym
    memo = initial
    self.my_each do |x|
      memo = memo.send(sym,x)
    end
    memo
  when initial && block_given?
    memo = initial
    self.my_each do |x|
      memo = yield(memo,x)
    end
    memo
  when block_given?
    memo = self[0]
    self[1..-1].my_each do |x|
      memo = yield(memo,x)
    end
    memo
  end
end

Each of the when statements is very similar. There are subtle differences but we don’t want to focus on that, we want to expose patterns. They all kind of look like this:

when some_condition
  memo = something
  something_or_other.my_each do |x|
    memo = some_combination_of_memo(var1, var2)
  end
  memo
end

To me, the most obvious place to start is with the starting memo. In two cases, it’s equal to initial, in the other two it’s self. Rather than defining that each time, let’s define it before we enter the when clauses.

def my_inject(initial = nil, sym = nil)
  memo =  if initial.is_a?(Symbol) || (!initial && block_given?)
            self[0]
          else
            initial
          end
  case 
  when initial.is_a?(Symbol)
    sym = initial
    self[1..-1].my_each do |x|
      memo = memo.send(sym, x)
    end
    memo
  when initial && sym
    self.my_each do |x|
      memo = memo.send(sym,x)
    end
    memo
  when initial && block_given?
    self.my_each do |x|
      memo = yield(memo,x)
    end
    memo
  when block_given?
    self[1..-1].my_each do |x|
      memo = yield(memo,x)
    end
    memo
  end
end

That’s cool. Each when clause has a call to my_each after either self or self[1..-1]. Why is it different each time? Set it once at the beginning.

def my_inject(initial = nil, sym = nil)
  memo =  if initial.is_a?(Symbol) || (!initial && block_given?)
            self[0]
          else
            initial
          end
  starting =  if initial.is_a?(Symbol) || (!initial && block_given?)
                self[1..-1]
              else
                self
              end
  case 
  when initial.is_a?(Symbol)
    sym = initial
    starting.my_each do |x|
      memo = memo.send(sym, x)
    end
    memo
  when initial && sym
    starting.my_each do |x|
      memo = memo.send(sym,x)
    end
    memo
  when initial && block_given?
    starting.my_each do |x|
      memo = yield(memo,x)
    end
    memo
  when block_given?
    starting.my_each do |x|
      memo = yield(memo,x)
    end
    memo
  end
end

We’re getting better but what do we have now? We have duplicate code right at the start! That can be fixed easily. We don’t want to set the memo and starting variables within the if statement, so let’s do it using the output of if. We can set multiple variables to elements of an array.

def my_inject(initial = nil, sym = nil)
  memo, starting =  if initial.is_a?(Symbol) || (!initial && block_given?)
                      [self[0], self[1..-1]]
                    else
                      [initial, self]
                    end
  case 
  when initial.is_a?(Symbol)
    sym = initial
    starting.my_each do |x|
      memo = memo.send(sym, x)
    end
    memo
  when initial && sym
    starting.my_each do |x|
      memo = memo.send(sym,x)
    end
    memo
  when initial && block_given?
    starting.my_each do |x|
      memo = yield(memo,x)
    end
    memo
  when block_given?
    starting.my_each do |x|
      memo = yield(memo,x)
    end
    memo
  end
end

Getting there. What pops up now? Hopefully that the last two when clauses are identical aside from their conditions. The conditions used to matter when we were setting variables within them. Since that’s not happening anymore, we can clean that up.

def my_inject(initial = nil, sym = nil)
  memo, starting =  if initial.is_a?(Symbol) || (!initial && block_given?)
                      [self[0], self[1..-1]]
                    else
                      [initial, self]
                    end
  case 
  when initial.is_a?(Symbol)
    sym = initial
    starting.my_each do |x|
      memo = memo.send(sym, x)
    end
    memo
  when initial && sym
    starting.my_each do |x|
      memo = memo.send(sym,x)
    end
    memo
  when block_given?
    starting.my_each do |x|
      memo = yield(memo,x)
    end
    memo
  end
end

What about that whole initial/sym thing? If not for that, the first two when clauses would be identical, so let’s declare sym before when and then we won’t need both of those.

def my_inject(initial = nil, sym = nil)
  memo, starting =  if initial.is_a?(Symbol) || (!initial && block_given?)
                      [self[0], self[1..-1]]
                    else
                      [initial, self]
                    end
  symbol = sym || initial
  case
  when initial
    starting.my_each do |x|
      memo = memo.send(symbol, x)
    end
    memo
  when block_given?
    starting.my_each do |x|
      memo = yield(memo,x)
    end
    memo
  end
end

This seems to make sense but we run our specs and… a failure!

1) inject with int and block 
     Failure/Error: memo = memo.send(symbol, x)
     TypeError:
       100 is not a symbol
     # ./refactor.rb:249:in `block in my_inject'
     # ./refactor.rb:5:in `my_each'
     # ./refactor.rb:248:in `my_inject'
     # ./refactor.rb:309:in `block (3 levels) in &top (required)>'
     # ./refactor.rb:310:in `block (3 levels) in &top (required)>'

What gives? Well, since our starting value can be a symbol or an integer, we need to approach that line we just added a bit differently. We have been looking for the presence of initial but maybe that’s not the best way of handling it. Since rspec is complaining about what we are feeding send, let’s focus on that. We need to determine if there is a symbol to send and, if so, what is it? Then we only want to send if there’s a symbol.

Along the way, we’re going to fix that whole case/when situation. Both of the remaining cases start and end the same way, so let’s wrap that around the conditions and then perform some logic inside.

def my_inject(initial = nil, sym = nil)
  memo, starting =  if initial.is_a?(Symbol) || (!initial && block_given?)
                      [self[0], self[1..-1]]
                    else
                      [initial, self]
                    end
  symbol = sym ? sym : (initial.is_a?(Symbol) ? initial : nil)
  starting.my_each do |x|
    memo = memo.send(symbol, x) if symbol
    memo = yield(memo, x) if block_given?
  end
  memo
end

We are aaaaalmost done. This new code is better but this line is kind of silly:


symbol = sym ? sym : (initial.is_a?(Symbol) ? initial : nil)

It’s certainly better than this:

if sym
  sym
else
  if initial.is_a?(Symbol)
    initial
  else
    nil
  end
end
But we can just use the   operator to handle some of logic. 
def my_inject(initial = nil, sym = nil)
  memo, starting =  if initial.is_a?(Symbol) || (!initial && block_given?)
                      [self[0], self[1..-1]]
                    else
                      [initial, self]
                    end
  symbol = sym || (initial.is_a?(Symbol) ? initial : nil)
  starting.my_each do |x|
    memo = memo.send(symbol, x) if symbol
    memo = yield(memo, x) if block_given?
  end
  memo
end

It’s still an important line to understand because it makes our line #317 possible

symbol = sym || (initial.is_a?(Symbol) ? initial : nil)

The key is that we’re declaring symbol, just like we declare initial and sym as defaulting to nil in the method. We need symbol to exist or this line will fail:

memo = memo.send(symbol, x) if symbol

We use the parenthesis to control the folow of logic, with the ternary operator reducing this:

if initial.is_a?(Symbol)
  initial
else
  nil
end

To a simple one-line expression.

expression_returning_boolean ? do_this_if_true : do_this_if_false

I was testing it and realized that we’ve left something out, haven’t we? We’re testing sym, int and sym, block, int and block, but what if someone gives int and block and sym? They shouldn’t do that, so let’s look for that right at the beginning.

def my_inject(initial = nil, sym = nil)
  raise 'Cannot pass int, sym, and block' if initial && sym && block_given?
  memo, starting =  if initial.is_a?(Symbol) || (!initial && block_given?)
                      [self[0], self[1..-1]]
                    else
                      [initial, self]
                    end
  symbol = sym || (initial.is_a?(Symbol) ? initial : nil)
  starting.my_each do |x|
    memo = memo.send(symbol, x) if symbol
    memo = yield(memo, x) if block_given?
  end
  memo
end

And we’ll modify our tests to check for that, too. Uncomment the tests below, comment out the tests at the bottom of the page to execute.

public :my_inject, :my_each
require 'rspec'

describe 'inject with' do
  let(:a) { [1, 2, 3, 4] }

  context 'sym' do
    subject { a.my_inject(:+) }
    it { is_expected.to eq a.inject(:+) }
  end

  context 'int and sym' do 
    subject { a.my_inject(100, :+) }
    it { is_expected.to eq a.inject(100, :+) }
  end

  context 'block' do
    subject { a.my_inject { |memo, x| memo + x } }
    it { is_expected.to eq a.inject { |memo, x| memo + x } }
  end

  context 'int and block' do
    subject { a.my_inject(100) { |memo, x| memo + x } }
    it { is_expected.to eq a.inject(100) { |memo, x| memo + x } }
  end

  context 'int, block, and sym' do
    it 'raises an error' do
      expect { a.my_inject(100, :+) { |memo, x| memo + x } }.to raise_error
    end
  end
end

And there you have it! From 27 lines down to 12. It could always be a bit more concise but for me, it’s perfectly readable and won’t require a ton of head-scratching if I ever have to revisit it.

Hope this helps. Get in touch if you have any questions, [email protected].

public :my_inject, :my_each
require 'rspec'

describe 'inject with' do
  let(:a) { [1, 2, 3, 4] }

  context 'sym' do
    subject { a.my_inject(:+) }
    it { is_expected.to eq a.inject(:+) }
  end

  context 'int and sym' do 
    subject { a.my_inject(100, :+) }
    it { is_expected.to eq a.inject(100, :+) }
  end

  context 'block' do
    subject { a.my_inject { |memo, x| memo + x } }
    it { is_expected.to eq a.inject { |memo, x| memo + x } }
  end

  context 'int and block' do
    subject { a.my_inject(100) { |memo, x| memo + x } }
    it { is_expected.to eq a.inject(100) { |memo, x| memo + x } }
  end
end