Block linking in Enumerable's each, allowed?

each, enumerable, ruby

Solution

Actually I think the following version is even more commonly seen:

def each
  @items.each { |i| yield i }
end

This is equivalent to your first code sample. There is however a subtle difference between this and your second version:

class Test
  def initialize(items)
    @items = items
  end

  def each1
    @items.each { |i| yield i }
  end

  def each2(&block)
    @items.each(&block)
  end
end

Observe:

irb(main):053:0> Test.new([1,2,3]).each2
=> #<Enumerator: [1, 2, 3]:each>
irb(main):054:0> Test.new([1,2,3]).each1
LocalJumpError: no block given (yield)
    from (irb):43:in `block in each1'
    from (irb):43:in `each'
    from (irb):43:in `each1'
    from (irb):54
    from /usr/bin/irb:12:in `<main>'

The version that delegates the block to the underlying iterable actually returns an enumerator if no block is given, which is very nice. It allows us to write stuff like this:

irb(main):055:0> Test.new([1,2,3]).each2.map { |x| x + 1 }
=> [2, 3, 4]

To achieve the same with our explicit version, we'd have to adapt it like this:

def each1
  return enum_for(:each1) unless block_given?
  @items.each { |i| yield i }
end

Which is even more verbose. Bottomline: Delegate the block to an underlying enumerable whenever possible, to save yourself from code duplication and from subtle gotchas like these.

By the way, now that you realize the two-folded nature of your `each` method, it would make sense to name it differently. For example, you could follow the example of methods like `String#chars` or `IO#lines` and call your method `items`:

def items(&block)
  @items.each(&block)
end

Problem

Original source