Passing objectified methods to iterators via &-syntax instead of code blocks

[rvashurin]

So I'd like to know communities' position on this issue. Say, we have a class that has several accessors:

class A
  ATTRS=%i(a b c d)
  attr_accessor :a, :b, :c, :d
   
  def initialize(:a, :b, :c, :d)
    @a, @b, @c, @d = a, b, c, d
  end
end

And at some point we might want to know if all the attributes are empty (suppose we know for sure they cannot be false). So we might add a method that would look like this:

def is_empty?
  ATTRS.none? { |attr| public_send(attr) }
end

Or we can dive into more obscure elements of Ruby and write it like this:

def is_empty?
  ATTRS.none?(&method(:public_send))
end

What's a good practice and a bad practice in this example, and why?

Another example might go like this. We have a class that contains a collection.

class B
  attr_accessor :collection
   
  def initialize(:collection)
    @collection = collection
  end
end

Collection may be represented in some other way, so we need a method that will transform it:

def transform_element(el)
  # do something with element
end

def transformed_collection
  @transformed ||= collection.map { |el| transform_element(el) }
end

Or, like in the first example:

def transform_element(el)
  # do something with element
end

def transformed_collection
  @transformed ||= collection.map(&method(:transform_element))
end

I strongly believe that a second approach is a wild overengineering, but I want to know what is a communities stance on this and whether this is a widely used way of doing things. If not, maybe it makes sense to add this type of thing to the guide?

P.S.

I do understand that both of these examples are very synthetic in the sense that at least second one may be resolved by moving transform_element method to the class of element of collection and re-writing method as:

def transformed_collection
  @transformed ||= collection.map(&:transform)
end

I just wanted to highlight this one way of doing things, just imagine that third way is not an option for some reason.

[xab3r]

ATTRS=%i(a b c d)
attr_accessor :a, :b, :c, :d

looks like the attributes are defined twice.

I would explicitly add a, b, c, d to a collection. #public_send makes the code less readable and hard to understand.

If doing the check is something really common in the project, I would use a method:

class A
  my_attr_accessor :a, :b, :c, :d
   
  def initialize(:a, :b, :c, :d)
    @a, @b, @c, @d = a, b, c, d
  end

  def is_empty?
    my_attributes.none?
  end
end

[kvokka]

my opinion is, that using any sort of send, method and all eval's must be reduced as much as possible. Of cause it will work, but is much less readable.

for sample

def foo(*a)
  p a
  a
end
('a'..'h').to_a.each_with_index.map &method(:foo) # => [["a", 0], ["b", 1], ["c", 2], ["d", 3], ["e", 4], ["f", 5], ["g", 6], ["h", 7]]

everything as expected, but really you have no idea, which arity will be send in the method at the moment. i prefer to show it explicit, like ('a'..'h').to_a.each_with_index.map{|*args| foo *args }. same length, but much readable result

[pirj]

In my book, there's only excuses for using &method.

It's when it's either hard to come up with clear parameter names:

chaos = ['A', -1, nil, {a: 1}]
chaos.map { |something| normalize(something) }

And when what is passed over doesn't matter to understand what is going on:

dimensions = [[1, 2], [5, 6]]
sizes = dimensions.each { |length, width| calculate(length, width) } 
# or
dimensions = [[1, 2, 3], [5, 6, 7]]
sizes = dimensions.each { |length, width, height| calculate(length, width, height) } 

Especially since this style of delegation is not available for blocks:

sizes = dimensions.each { |(*)| calculate(*) } # Ouch, even in 3.0
sizes = dimensions.each { |(*dim)| calculate(*dim) } # Works

does not leave much choice.

[marcandre]

In my book, there's only excuses for using &method.

It's when it's either hard to come up with clear parameter names:

chaos = ['A', -1, nil, {a: 1}]
chaos.map { |something| normalize(something) }

Why? Just use _1:

chaos.map { normalize(_1) }

And when what is passed over doesn't matter to understand what is going on:

dimensions = [[1, 2], [5, 6]]
sizes = dimensions.each { |length, width| calculate(length, width) } 
# or
dimensions = [[1, 2, 3], [5, 6, 7]]
sizes = dimensions.each { |length, width, height| calculate(length, width, height) } 

No sure what the issue is, or why you'd want to use the implicit splatting of each here. Why not write:

dimensions.each { |dim| calculate(*dim) } 

FWIW, dimensions would be best renamed as dimensions_list

I don't know of any good reason to call &method. FWIW, performance of that is quite bad.

[pirj]

chaos.map { normalize(_1) }

You have to specify the number of arguments to pass. As you can see in the second example there can be either two or three, and the code that binds dimensions_list to calculate doesn't have and doesn't have to have awareness of the number of arguments.

I would imagine it could be possible to pass through an arbitrary number of arguments:

chaos.map { normalize(*) }
# or
chaos.map { normalize(*_) }

And I don't think I will ever get used to _1, nor find it useful to improve the readability.

dimensions_list.map { |dimensions| calculate(*dimensions) }

:+1: That looks quite good. I regret using the list as an example. My point was to demonstrate that it doesn't matter what parameters are, and it boils down to the same as the above example.

dimensions_list.map { calculate(*) }
# or
dimensions_list.map { calculate(*_) }

I hope you'd agree that those (imaginable, wishful thinking) examples are better than:

chaos.map(&method(:normalize))

dimensions_list.map(&method(:calculate))

because it was my only point to demonstrate that in the case when the binding code has no awareness of the composition and purpose of arguments passed through, the usage of &method is a reasonable choice.

Unfortunately, I have no awareness of where Ruby is going with this syntax, or if an implicit *_ or * will ever be available. From my point of view, it would fit nicely in the above cases to replace &method.

[marcandre]

Where I think we don't agree is that yielding is usually 1 element. Even Hash#each has been modified to yield [key, value] instead of 2 elements. This is not typically obvious, and this is made less obvious by the fact that {|x|...} is not the same as {|x, |...}, or that { _1; ... } does not behave the same if there is _2 in the ... or not.

It's not clear from your text if you realize that dimensions_list.map(&method(:calculate)) will call calculate with a single argument (which will be an array of dimensions)

[pirj]

My bad, I've missed the "to iterators" part of the title of this discussion.

Should we consider .with_index/.with_object/inject when talking about &method? max/min/sort? slice_when/chunk_while? zip? Chained chunk?

Sorry for cleverly evading the "if you realize" question :laughing: I did this to protect my ego and the illusion that I really dig in this kind of stuff.

[marcandre]

😆

Should we consider .with_index/.with_object/inject when talking about &method? max/min/sort? slice_when/chunk_while? zip? Chained chunk?

I don't think so. There's only a single case of &method in RuboCop and it's not bad.