How I refactor, based on http://www.reddit.com/r/ruby/comments/2h0g7f/code_reviewrecreated_some_of_the_enumerable/

refactor.rb

# /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.

# 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

# 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. This is really the crux of what we're going to be doing:
# finding patterns that are almost identical, extracting the snowflake portion into variables, 
# and then sending those variables forward.

# 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.
# An easy pitfall here is to do it as `if`/`else`. `when` is more `if`/`if`, so we'll evaluate each one independently
# to give the code an opportunity to build to a resolution.

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 flow 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, chris@subvertallmedia.com.

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