Crystal: Array#reject! shouldn't return Nil

[tbrand]

Reproducible code

arr = [ 1, 2, 4, 5 ]
arr
  .reject! { |e| e%2 == 0 }
  .reject! { |e| e == 1 }

puts arr

When execute this code in ruby, it returns [5].
In Crystal, it raises a following error.

Error in filename:2: undefined method 'reject!' for Nil (compile-time type is (Array(Int32) | Nil))

The problem is not the compatibility, but the usability.
In many real cases, we use such methods in chains.
So reject! should return an empty array when reject! removes all of the elements.
Needless to say, using not_nil! is not safety.

Thanks

Crystal version: 0.23.1
Sierra 10.12.6

[vladfaust]

arr = [1, 2, 4, 5]
arr = arr
  .reject { |e| e % 2 == 0 }
  .reject { |e| e == 1 }

puts arr # => [5]

This shouldn't cause any sensible performance impact.

[tbrand]

Hmm...
OK, I'll modify my code like that.:+1:

[asterite]

@tbrand Actually, Crystal is safer here. If arr is not known to you, this can happen:

# Ruby
arr = [1, 5]
arr
  .reject! { |e| e % 2 == 0 }
  .reject! { |e| e == 1 }

puts arr

Output:

bar.cr:4:in `<main>': undefined method `reject!' for nil:NilClass (NoMethodError)

Exactly the same error Crystal gives you, but Ruby tells you at runtime time, Crystal at compile time.

There's no need to use reject, there's always try:

arr = [1, 2, 4, 5]
arr
  .reject! { |e| e % 2 == 0 }
  .try &.reject! { |e| e == 1 }

puts arr

[tbrand]

Ruby tells you at runtime time, Crystal at compile time.

💯

There's no need to use reject, there's always try:

try is the best!! Thanks!! (I forgot the existence...)

[oprypin]

Hold on... why would reject! ever return nil? That makes no sense. (Yes, it's described in the documentation, but why do that?)

[asterite]

That way you know if reject! made changes or not.

[oprypin]

OK but there are other ways to get this information. Other methods don't have such a quirk.
So, why do that?

[asterite]

Carried from Ruby. How else would you do that? Maybe that info is not important...?

[oprypin]

Well, the obvious thing is to return self, as almost every other ! method does. But whatever, this is not worth arguing about.

[oprypin]

By the way, how to get that information -- get size before and size after

[asterite]

I think it's better to always return self so chaining is possible. It's true that the return value is almost meaningless.

[AlexWayfer]

Now we need the same changes in Ruby :sweat_smile:
(I'm sorry, a sore subject for me)