Updates:
- Brian Hogan did a talk on "don't reinvent the wheel" at the 2010 Ruby Hoedown; he has more examples around
FileUtils
andPathname
and 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#tap
but doesn't. - David Chelimsky suggested that
foo.map{}.compact
can be replaced withfoo.select{}.map{}
(similarlyfoo.select{}.compact
indicates that theselect
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 otherMatchData
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:
This can be replaced with:
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:
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:
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:
This is still closer to what I'm aiming for.
Can you think of any more? Send them to @tcopeland
.