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 Enumerable#in_slices_of #13108

Merged

Conversation

pricelessrabbit
Copy link
Contributor

@pricelessrabbit pricelessrabbit commented Feb 23, 2023

Adding missing method and related tests

Resolves #12844

src/enumerable.cr Outdated Show resolved Hide resolved
src/enumerable.cr Outdated Show resolved Hide resolved
Co-authored-by: Sijawusz Pur Rahnama <sija@sija.pl>
@straight-shoota straight-shoota changed the title added in_slices_of method to enumerable Added Enumerable#in_slices_of Feb 24, 2023
it { [1, 2, 3].in_slices_of(1).should eq([[1], [2], [3]]) }
it { [1, 2, 3].in_slices_of(2).should eq([[1, 2], [3]]) }
it { [1, 2, 3, 4].in_slices_of(3).should eq([[1, 2, 3], [4]]) }
it { ([] of Int32).in_slices_of(2).should eq([] of Array(Array(Int32))) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong type:

Suggested change
it { ([] of Int32).in_slices_of(2).should eq([] of Array(Array(Int32))) }
it { ([] of Int32).in_slices_of(2).should eq([] of Array(Int32)) }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me wish for an equality expectation that also takes type into account...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing the point about the wrong type when array is empty. shouldn't it be the same as cases where some item is present?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.should be_empty?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[] of Array(Array(Int32)) is a three-dimensional structure, whereas the method should only return a two-dimensional structure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right! type is wrong also fot other methods like #in_groups_of and this misled me.

So how i should proceed?

  • keep the [] of Array(Array(Int32)) form or change to .should be_empty?
  • can i search and fix also tests of other method in this PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think making something about this is important. I suggest we merge this and maybe capture a different issue to unify the code.

Copy link
Member

@straight-shoota straight-shoota Apr 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a new issue to continue the discussion about stricter type checking for equality expectations: #13389. Changing any other specs is out of scope here.

For this PR I think it's best to apply the suggested change to [] of Array(Int32). Then this should be ready to merge.

@straight-shoota straight-shoota added this to the 1.9.0 milestone May 8, 2023
@straight-shoota straight-shoota merged commit c5b81b8 into crystal-lang:master May 9, 2023
@straight-shoota straight-shoota changed the title Added Enumerable#in_slices_of Add Enumerable#in_slices_of Jul 2, 2023
This pull request was closed.
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.

Array form of #each_slice
5 participants