Refactor to use Ruby standard library

18 Sep 2013

Updates:

  • Brian Hogan did a talk on "don't reinvent the wheel" at the 2010 Ruby Hoedown; he has more examples around FileUtils and Pathname and such. For example, replacing File.read(File.join(Rails.root, "config", "database.yml") with Rails.root.join("config", "database.yml").read.
  • Bruce Williams notes that folks may not be aware of and thus neglect newer Ruby methods like Enumerable#each_with_object.
  • Nathan Long said that there's an antipattern around code that should use Kernel#tap but doesn't.
  • David Chelimsky suggested that foo.map{}.compact can be replaced with foo.select{}.map{} (similarly foo.select{}.compact indicates that the select block should be filtering out nils).
  • Another example I thought of later: replace [1,2,3].select {|x| x > 1}.size with [1.2.3].count {|x| x > 1}.
  • And another: replace [1,2,3].select {|x| x > 2 }.size == 1 with [1,2,3].one? {|x| x > 2 }. There are a bunch of variants on that one, like replace [1,2,3].select {|x| x > 2}.size > 0 with [1,2,3].any? {|x| x > 2}.
  • Replace ![1,2,3].detect {|x| x > 2}.nil? with [1,2,3].any? {|x| x > 2}. This is like the one above except this transformation makes it clear that we're checking for the presence of something vs needing the thing itself.
  • Replace "foobar".match(/^foo/) with "foobar".start_with?("foo"). There are a variants on this; "foobar".match(/bar$/) can be replaced with "foobar".end_with?("bar"), a few different possibilities around checking for substrings, etc. The challenge here is to ensure that the regex match is only being done for the existence check; if the code is using captures or prematch or some other MatchData feature then we can't do this simplification. James Edward Grey II has a bunch of variants of this on his Regex Code Equivalency post.
  • Chris Morris noticed that [1,2,3].shuffle.first can be replaced with [1,2,3].sample or [1,2,3].choice in Ruby 1.8.
  • Replace [1,2,3].map {|x| [x,x+1] }.flatten(1) with [1,2,3].flat_map {|x| [x,x+1] }
  • From Bozhider Batsov's Ruby Style Guide - replace [1,2,3].reverse.each {|x| puts x } with [1,2,3].reverse_each {|x| puts x }.

Sometimes I see code that does some unnecessary computation simply because it doesn't use the Ruby standard library effectively:

widgits = get_widgits.select {|w| w.cromulent? }.first

This can be replaced with:

widgits = get_widgits.detect {|w| w.cromulent? }

Not a big deal, but it saves a few opcodes and more clearly shows the intent. Of course, this refactoring only works if the cromulent? method that's called in the block doesn't have side effects.

I'm trying to think of more cases like this. Maybe they're called "ruby standard library micro antipatterns" or "ruby un-idioms" or something. A difficulty is that there are so many ways to write code that doesn't fully take advantage of the standard library (which, as Michael Evans noted, is harder in Ruby because you actually have to read the docs rather than relying on typing a dot and waiting for the method list to pop up). So I'm having trouble zeroing in on a set of common instances.

There are control flow antipatterns like this one described by Stephen Chen, where the code can be simplified:

# replace this with:
# return foo && bar
if foo && bar
  true
else
  false
end

That sort of thing could be detected with static analysis. But I'm more interested in library misuse, not language misuse. Here's an actual bug; this is not quite what I'm looking for, but closer:

x = [1,2,3]
x.reject {|y| y > 2 } # oops meant to use reject!
here_are_some_small_numbers(x)

This is "discarded return value" and we could catch it with static analysis. Along the same lines, in the example below the return value of strip! is being used unnecessarily:

x = "foo "
x = x.strip!  # can simplify by removing the assignment

This is still closer to what I'm aiming for.

Can you think of any more? Send them to @tcopeland.