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 `Iterator#slice_before` #7152

Merged
merged 1 commit into from Dec 7, 2018

Conversation

Projects
None yet
4 participants
@asterite
Copy link
Contributor

asterite commented Dec 6, 2018

Part of #7142

This also improves the documentation of slice_after, now the two docs are similar and make it more clear why they are called that way.

I thought about reusing the implementation of slice_before but it might not be worth it. The algorithms need different variables. Plus at least in Ruby I think they don't seem to reuse the algorithm either. (in Ruby it's also much, much longer, in Crystal the core algorithm is just 30 lines)

@j8r

This comment has been minimized.

Copy link
Contributor

j8r commented Dec 6, 2018

Seems you are liking naming arrays ary - we are wondering why 😄

@asterite

This comment has been minimized.

Copy link
Contributor Author

asterite commented Dec 6, 2018

ary is short for Array. Ruby uses it in its examples. For example: https://ruby-doc.org/core-2.2.0/Range.html#method-i-bsearch

It also happens to be my name :-P

# of chunks are defined by the block, when the block's value
# is _truthy_. Thus, the enumerable is sliced just **after** each
# time the block returns a _truthy_ value.
# Returns an iterator for each chunked elements where each chunk

This comment has been minimized.

@RX14

RX14 Dec 7, 2018

Member

Perhaps

Returns an iterator over chunks of elements, where each chunk ends right after the given block's value is truthy

This comment has been minimized.

@asterite

asterite Dec 7, 2018

Author Contributor

Done!

@@ -1377,4 +1376,96 @@ module Iterator(T)
self
end
end

# Returns an iterator for each chunked elements where each chunk
# ends right **before** the given block's value is _truthy_.

This comment has been minimized.

@RX14

RX14 Dec 7, 2018

Member

either bold before above or remove it here

This comment has been minimized.

@asterite

asterite Dec 7, 2018

Author Contributor

Done!

end
end

if @block.call(value) && !@values.empty?

This comment has been minimized.

@RX14

RX14 Dec 7, 2018

Member

!@values.empty? is cheaper and should go before the @block.call. The optimizer won't be able to reorder as it won't be able to prove that @block doesn't modify @values.

This comment has been minimized.

@asterite

asterite Dec 7, 2018

Author Contributor

Good idea!

@asterite asterite force-pushed the asterite:feature/slice_before branch from c62bff8 to 20a23cf Dec 7, 2018

@RX14

RX14 approved these changes Dec 7, 2018

@RX14

This comment has been minimized.

Copy link
Member

RX14 commented Dec 7, 2018

Might be good to hold off merging this until we reach consensus on what happens with non-empty reuse: though.

@asterite

This comment has been minimized.

Copy link
Contributor Author

asterite commented Dec 7, 2018

But we have reuse in many other methods and they behave the same.

We can probably tackle the reuse "issue" in a separate discussion.

@asterite

This comment has been minimized.

Copy link
Contributor Author

asterite commented Dec 7, 2018

Plus we already merged slice_after

@RX14 RX14 merged commit 53ed9e9 into crystal-lang:master Dec 7, 2018

4 checks passed

ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@RX14 RX14 added this to the 0.27.1 milestone Dec 7, 2018

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