Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add with_object with block to iterator #6043

Closed
wants to merge 1 commit into from

Conversation

esse
Copy link
Contributor

@esse esse commented May 1, 2018

This PR adds missing with_object with block option to iterator (same as f.e. with_index with block).

@esse esse force-pushed the add_with_object_to_iterator branch from 0b754cc to 7a96a9e Compare May 1, 2018 20:14
@straight-shoota
Copy link
Member

straight-shoota commented May 1, 2018

What would this be useful for? If you're iterating with yield, you can just use directly access the object from the block scope. with_object is only necessary when it returns an iterator which wraps the object so that it can be accessible in a different scope.

iterator.with_object(my_with_object) do |obj, elem|
  do_something_with obj, elem
end
# vs.
iterator.each do |elem|
  do_something_with my_with_object, elem
end

This method overload leads to longer and more complicated code. I don't think this makes sense.

@esse
Copy link
Contributor Author

esse commented May 1, 2018

I believe we should have this for the sake of consistency - right now we have with_index which accepts a block, and with_object that doesn't.

In Ruby both these method accept blocks and I think that iterator method that doesn't accept block is quite misleading.

@esse esse force-pushed the add_with_object_to_iterator branch from 7a96a9e to f024e2b Compare May 1, 2018 21:18
@straight-shoota
Copy link
Member

iterator method that doesn't accept block is quite misleading.

how so?

There are lots of methods in Iterator that don't accept a block but only return an iterator and vice versa. Each is available where it makes sense. with_index is useful with a block, because it implements the index counter for you. It saves effort and makes the code more concise. with_object with block is the opposite.

@jkthorne
Copy link
Contributor

jkthorne commented May 1, 2018

I think having methods with each in the name accept a block is a good idea.

@straight-shoota
Copy link
Member

I don't think this feature provides a reasonable value. Using this method has literally no benefit over not using it. The argument for the addition was mainly consistency in method names, but the use cases for yielding methods differ from those returning an iterator. There is no strict demand for having equal interfaces for both.

Thank you @esse for your contribution nevertheless. I'm sorry to reject this, but I think this is the right decision.

(Feel free to object or reopen should I have missed an important argument.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants