Updates:
- Brian Hogan did a talk on "don't reinvent the wheel" at the 2010 Ruby Hoedown; he has more examples around
FileUtilsandPathnameand such. For example, replacingFile.read(File.join(Rails.root, "config", "database.yml")withRails.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#tapbut doesn't. - David Chelimsky suggested that
foo.map{}.compactcan be replaced withfoo.select{}.map{}(similarlyfoo.select{}.compactindicates that theselectblock should be filtering out nils). - Another example I thought of later: replace
[1,2,3].select {|x| x > 1}.sizewith[1.2.3].count {|x| x > 1}. - And another: replace
[1,2,3].select {|x| x > 2 }.size == 1with[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 > 0with[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 otherMatchDatafeature 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.firstcan be replaced with[1,2,3].sampleor[1,2,3].choicein 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? }.firstThis 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
endThat 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 assignmentThis is still closer to what I'm aiming for.
Can you think of any more? Send them to @tcopeland.