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

Implementing ArrayEachLimited method. #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ivahaev
Copy link

@ivahaev ivahaev commented Aug 10, 2016

I'm not sure of the name, but just wrote implementation of ArrayEach with ability to stop processing array when required value found. To keep the backward compatibility I create new method ArrayEachLimited and ArrayEach uses it internally.

So callback function receives one extra argument – index of element in array and should return bool. If called callback returns false, ArrayEachLimited will stop processing.

What do you think?

If called callback returns false, ArrayEachLimited will stop processing.
@daboyuka
Copy link
Contributor

In my opinion, it should return an error instead, and that error should be passed through to the caller of ArrayEachLimited.

@ivahaev
Copy link
Author

ivahaev commented Aug 11, 2016

I thought about error, but in my opinion, error need to return when something went wrong. In this case we just want to tell that no need to iterate more.

@daboyuka
Copy link
Contributor

True, but error is more versatile. You could return io.EOF to signal "done iterating, no problem" to your outer code.

@ivahaev
Copy link
Author

ivahaev commented Aug 11, 2016

Sounds good. Waiting for Leonid vote

@ivahaev
Copy link
Author

ivahaev commented Dec 17, 2016

So, Leonid, what do you think about it?

@ppanyukov
Copy link

I think I would prefer to return error, with io.EOF being a clean signal to stop.

@jimmyjames85
Copy link

Currently you can early exit in ObjectEach and I think adding that ability to ArrayEach would also be a good idea. Has there been any progress to resolve the branch conflicts? Are we still waiting @buger to vote?

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

Successfully merging this pull request may close these issues.

4 participants