i don't like tap

1-mentalguy.rb

# from http://moonbase.rydia.net/mental/blog/programming/eavesdropping-on-expressions
# For debugging and getting a puts in there, tap is nice
def blah
  @things.map { |x|
    x.length
  }.inject( 0 ) { |a, b|
    a + b
  }
end


#puts without tap
def blah
  sum = @things.map { |x|
    x.length
  }.inject( 0 ) { |a, b|
    a + b
  }
  p sum
  sum
end

#puts with tap
def blah
  @things.map { |x|
    x.length
  }.inject( 0 ) { |a, b|
    a + b
  }.tap { |sum| p sum }
end

2-rubybestpractices.rb

# from http://blog.rubybestpractices.com/posts/gregory/011-tap-that-hash.html
#
# all have the same number of lines. which is the most clear? I would say "ugly"

def ugly
  results = {}
  [:x, :y, :z].each do |letter|
    results[letter] = rand(100)
  end
  results
end

def refactored_returning
  returning({}) do |results|
    [:x, :y, :z].each do |letter|
      results[letter] = rand(100)
    end
  end
end

def refactored_tap
  {}.tap do |results|
    [:x, :y, :z].each do |letter|
      results[letter] = rand(100)
    end
  end
end

## toothrot:
def refactored_inject
  [:x, :y, :z].inject({}) do |results, letter|
    results[letter] = rand(100)
    results
  end
end

3-rubydocs.md

Even the RubyDocs recommend it only for puts!

http://www.ruby-doc.org/core-1.9/classes/Object.html#M000191

Yields x to the block, and then returns x. The primary purpose of this method is to "tap into" a method chain, in order to perform operations on intermediate results within the chain.

(1..10)                .tap {|x| puts "original: #{x.inspect}"}
  .to_a                .tap {|x| puts "array: #{x.inspect}"}
  .select {|x| x%2==0} .tap {|x| puts "evens: #{x.inspect}"}
  .map { |x| x*x }     .tap {|x| puts "squares: #{x.inspect}"}

4-hilariously-bad.rb

# http://www.tonyamoyal.com/2011/01/28/more-cool-stuff-with-rubys-tap-method/
#
# aaaand this is why i really hate it

class AssetDeliveryPopulater
 
  def self.populate
    results_text = []
    asset_secrets_count, asset_delivery_count, asset_not_found_count, asset_delivery_creation_errors = 0,0,0,0
 
    Email.all.tap{|emails| results_text << "#{emails.length} emails parsed." }.each do |e|
      next unless e.body
 
      asset_secrets = extract_asset_secrets_from_text(e.body)
      next unless asset_secrets
 
      asset_secrets.tap{|as| asset_secrets_count += as.length}.each do |asset_secret|
        if a = Asset.find_by_secret(asset_secret)
          if ad = AssetDelivery.create(
              :asset => a,
              :campaign => e.campaign,
              :client => e.client,
              :created_at => e.created_at
            )
            asset_delivery_count += 1
            puts "Recorded an asset delivery: #{ad.inspect}"
          else
            asset_delivery_creation_errors += 1
            puts "Could not record the delivery: #{ad.errors.inspect}"
          end
        else
          asset_not_found_count += 1
          puts "Could not find asset with secret #{asset_secret}"
        end
      end
    end
 
    results_text << "#{asset_secrets_count} asset secrets found."
    results_text << "#{asset_delivery_count} asset deliveries recorded."
    results_text << "#{asset_not_found_count} assets not found."
    results_text << "#{asset_delivery_creation_errors} asset delivery records not created due to errors."
    puts results_text.join("\n")
  end
 
  # Given some text, extract secret asset hashes and return an array
  def self.extract_asset_secrets_from_text(text)
    text.scan(/http:\/\/[^'\/]+\/products\/([^'\/]+)/).first
  end
end

For the second example, I think I'd even prefer:

def refactored_without_tap(results = {})
  [:x, :y, :z].each do |letter|
    results[letter] = rand(100)
  end
  results
end

Essentially the same as ugly, but suggests the Extract Method I'd do if there were other behaviors in the original method.

I don't think the responsibility for doing that yield/return dance really lies with any given Object. So far, I think I'd almost always rather extract a method, unless it's doing something like MenTaLguY's original implementation (first example), just for debugging purposes. Leaving it there for production code is a bit of a harder sell for me.

oh! Yeah, none of that code was actually mine. I absolutely agree with you on all points there.

I actually prefer "ugly". I think being explicit is paramount, especially after years of working on large codebases.

Yeah, I knew those were mostly from the links, and that we're pretty much on the same page already. Just wanted to put my 2 cents in. I'm curious to see what @patmaddox's case for tap is...