pragmatist
Patrick Joyce

January 17, 2013

reject bang considered harmful

reject! has some (at least to me) unexpected behavior that can easily lead to bugs.

In the case where it makes changes to the array it returns the new, filtered value of the array, just like reject. However, in the case that it makes no changes it returns nil

>> a = [1, 2, 3, 4]
=> [1, 2, 3, 4]
>> a = a.reject(&:odd?)
=> [2, 4]
>> a
=> [2, 4]
>> a = a.reject(&:odd?)
=> [2, 4]
>> a
=> [2, 4]
>> a = [1, 2, 3, 4]
=> [1, 2, 3, 4]
>> a.reject!(&:odd?)
=> [2, 4]
>> a
=> [2, 4]
>> a.reject!(&:odd?)
=> nil
>> a
=> [2, 4]

As long as you don't store the return value or chain a method call on the reject! call you're fine.

However, this is one of those rare cases where I think it is worthwhile to defensively code for potential future changes. I've seen several bugs introduced in production code where people come back later and start chaining method calls a la @collection.reject!(&:odd?).each{|d| ...

Therefore, even though it is more verbose I prefer to use the form @collection = @collection.reject(&:odd?)

More Articles on Software & Product Development

Agile With a Lowercase “a”
”Agile“ is an adjective. It is not a noun. It isn’t something you do, it is something you are.
How Do You End Up With A Great Product A Year From Now?
Nail the next two weeks. 26 times in a row.
Build it Twice
Resist the urge to abstract until you've learned what is general to a class of problems and what is specific to each problem.