According to wikipedia, after Dijkstra’s letter Go To Statement Considered Harmful there were at least 65 papers published with titles like ‘X Considered Harmful’, most of them generating heat but little light. I’m using the convention ironically because, well, this is just a nit. It shouldn't be controversial all. :-)

Variable capture is the use of an externally defined variable inside of a closure or block. It's something you'll see in most code that uses closures, and often it's necessary. It’s just that given a better choice I’d rather not use it. It muddies code.

Let's look at some examples.

In this function, we don’t have it. Each stage in our pipeline uses only the data it has available to it from the previous stage.

def refactoring_reduction_profile events
  events.group_by(&:method_name)
        .map {|_,e| percent_reduction(e) }
        .freq_by {|e| (e * 100 / 10).to_i  }
end

In the following code, we do have variable capture. The select call uses the variable class_name. It’s a variable that does not come from the previous stage (in this case the events variable).

def active_months events, class_name
  events.select {|e| e.class_name == class_name }
        .map {|e| e.date.month_start }
        .sort
        .uniq
end

What’s the big deal? Well, in a case this small there isn’t much of a problem but as we introduce more variable capture, readability suffers:

def life_lines method_names, events
  method_groups = events.group_by(&:method_names)
  life_lines = method_names.map do |name| 
      method_groups[name].map(&:method_length)
  end
  max = life_lines.map(&:count).max
  make_chart(method_names, 
             life_lines.map {|line| line.adjusted_to(max) }.transpose)
end

Locality is one of the most important conditions for understandability in code. It’s why we favor small classes and methods. The funny thing is, as you start using a more compositional style and your code becomes more concise, your sensitivity to clutter increases. Or, at least mine does. I’d rather see a chain of calls with each receiving only data from the previous call.

In Martin Fowler’s Refactoring book, he makes the case that temporary variables bind long methods together. They make them harder to decompose. When they're accessed in blocks, they present the same problem that global variables present but in a much narrower context. It’s distracting to think about the outer scope when trying to understand an inner scope.

But, that said, in many cases, variable capture is unavoidable. In the last example, we could extract functions to make the code clearer, but unless we are using lazy evaluation or memoization we'll be making our code much less efficient. And, we still will not have eliminated all the capture.

def life_lines method_names, events
  lines = method_names.map { |name| method_lifelines(name) }
  make_chart(method_names, 
             lines.map {|l| l.adjusted_to(lines_max(lines)) }.transpose)
end

So.. variable capture considered harmful? Maybe. Completely avoidable? Probably not. I just find that I get a lot of value out of factoring toward pure pipeline style. It makes reasoning easier.