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?)