From 1097872f9c24dd70c7687b5ffee2d4fc1b3691e1 Mon Sep 17 00:00:00 2001 From: Philip Ross Date: Thu, 29 Jun 2023 10:35:09 -0700 Subject: [PATCH 1/8] implemented Enumerable#each_step and Iterable#each_step --- spec/std/enumerable_spec.cr | 34 ++++++++++++++++++++++++++++++++++ src/enumerable.cr | 36 ++++++++++++++++++++++++++++++++++++ src/iterable.cr | 14 ++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/spec/std/enumerable_spec.cr b/spec/std/enumerable_spec.cr index 3b3cef2bd0c1..c60e38b60a5a 100644 --- a/spec/std/enumerable_spec.cr +++ b/spec/std/enumerable_spec.cr @@ -393,6 +393,40 @@ describe "Enumerable" do end end + describe "each_step", focus: true do + it "yields the every 2nd element" do + collection = [] of String + ["a", "b", "c", "d", "e", "f"].each_step(2) do |e| + collection << e + end + collection.should eq ["a", "c", "e"] + end + + it "accepts an optional offset parameter" do + collection = [] of String + ["a", "b", "c", "d", "e", "f"].each_step(2, 1) do |e| + collection << e + end + collection.should eq ["b", "d", "f"] + end + + it "gets each_step iterator" do + iter = ["a", "b", "c", "d", "e", "f"].each_step(2) + iter.next.should eq("a") + iter.next.should eq("c") + iter.next.should eq("e") + iter.next.should be_a(Iterator::Stop) + end + + it "gets each_step iterator with an offset" do + iter = ["a", "b", "c", "d", "e", "f"].each_step(2, 1) + iter.next.should eq("b") + iter.next.should eq("d") + iter.next.should eq("f") + iter.next.should be_a(Iterator::Stop) + end + end + describe "each_with_index" do it "yields the element and the index" do collection = [] of {String, Int32} diff --git a/src/enumerable.cr b/src/enumerable.cr index 7d95cb6f81b2..ad67c83fc92a 100644 --- a/src/enumerable.cr +++ b/src/enumerable.cr @@ -426,6 +426,42 @@ module Enumerable(T) nil end + # Iterates over the collection, yielding every *n*th element, starting with the first. + # + # ``` + # ["Alice", "Bob", "Charlie", "David"].each_step(2) do |user| + # puts "User: #{user}" + # end + # ``` + # + # Prints: + # + # ```text + # User: Alice + # User: Charlie + # ``` + # + # Accepts an optional *offset* parameter + # + # ``` + # ["Alice", "Bob", "Charlie", "David"].each_step(2, 1) do |user| + # puts "User: #{user}" + # end + # ``` + # + # Which would print: + # + # ```text + # User: Bob + # User: David + # ``` + def each_step(n : Int, offset : Int = 0, & : T ->) : Nil + offset_mod = offset % n + each_with_index do |elem, i| + yield elem if i >= offset && i % n == offset_mod + end + end + # Iterates over the collection, yielding both the elements and their index. # # ``` diff --git a/src/iterable.cr b/src/iterable.cr index 86351bba0e70..73beaf97f441 100644 --- a/src/iterable.cr +++ b/src/iterable.cr @@ -51,6 +51,20 @@ module Iterable(T) each.cons_pair end + # Same as `each.step(n)`. + # + # See also: `Iterator#step`. + def each_step(n : Int) + each.step(n) + end + + # Same as `each.skip(offset).step(n)`. + # + # See also: `Iterator#skip`, `Iterator#step`. + def each_step(n : Int, offset : Int) + each.skip(offset < 0 ? offset % n : offset).step(n) + end + # Same as `each.with_index(offset)`. # # See also: `Iterator#with_index(offset)`. From 78023fed7a94fee57b603ae302714333f81dc1e1 Mon Sep 17 00:00:00 2001 From: Philip Ross Date: Thu, 29 Jun 2023 10:37:40 -0700 Subject: [PATCH 2/8] remove focus from spec --- spec/std/enumerable_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/std/enumerable_spec.cr b/spec/std/enumerable_spec.cr index c60e38b60a5a..313cad6eb490 100644 --- a/spec/std/enumerable_spec.cr +++ b/spec/std/enumerable_spec.cr @@ -393,7 +393,7 @@ describe "Enumerable" do end end - describe "each_step", focus: true do + describe "each_step" do it "yields the every 2nd element" do collection = [] of String ["a", "b", "c", "d", "e", "f"].each_step(2) do |e| From 71ceda69234d714488b2c30304e415d43c57833a Mon Sep 17 00:00:00 2001 From: Philip Ross Date: Thu, 29 Jun 2023 12:29:00 -0700 Subject: [PATCH 3/8] make offset a named parameter. update array generation --- spec/std/enumerable_spec.cr | 8 ++++---- src/enumerable.cr | 6 +++--- src/iterable.cr | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/std/enumerable_spec.cr b/spec/std/enumerable_spec.cr index 313cad6eb490..39881383c28b 100644 --- a/spec/std/enumerable_spec.cr +++ b/spec/std/enumerable_spec.cr @@ -396,7 +396,7 @@ describe "Enumerable" do describe "each_step" do it "yields the every 2nd element" do collection = [] of String - ["a", "b", "c", "d", "e", "f"].each_step(2) do |e| + %w[a b c d e f].each_step(2) do |e| collection << e end collection.should eq ["a", "c", "e"] @@ -404,14 +404,14 @@ describe "Enumerable" do it "accepts an optional offset parameter" do collection = [] of String - ["a", "b", "c", "d", "e", "f"].each_step(2, 1) do |e| + %w[a b c d e f].each_step(2, offset: 1) do |e| collection << e end collection.should eq ["b", "d", "f"] end it "gets each_step iterator" do - iter = ["a", "b", "c", "d", "e", "f"].each_step(2) + iter = %w[a b c d e f].each_step(2) iter.next.should eq("a") iter.next.should eq("c") iter.next.should eq("e") @@ -419,7 +419,7 @@ describe "Enumerable" do end it "gets each_step iterator with an offset" do - iter = ["a", "b", "c", "d", "e", "f"].each_step(2, 1) + iter = %w[a b c d e f].each_step(2, offset: 1) iter.next.should eq("b") iter.next.should eq("d") iter.next.should eq("f") diff --git a/src/enumerable.cr b/src/enumerable.cr index ad67c83fc92a..c4b78448af7a 100644 --- a/src/enumerable.cr +++ b/src/enumerable.cr @@ -429,7 +429,7 @@ module Enumerable(T) # Iterates over the collection, yielding every *n*th element, starting with the first. # # ``` - # ["Alice", "Bob", "Charlie", "David"].each_step(2) do |user| + # %w[Alice Bob Charlie David].each_step(2) do |user| # puts "User: #{user}" # end # ``` @@ -444,7 +444,7 @@ module Enumerable(T) # Accepts an optional *offset* parameter # # ``` - # ["Alice", "Bob", "Charlie", "David"].each_step(2, 1) do |user| + # %w[Alice Bob Charlie David].each_step(2, 1) do |user| # puts "User: #{user}" # end # ``` @@ -455,7 +455,7 @@ module Enumerable(T) # User: Bob # User: David # ``` - def each_step(n : Int, offset : Int = 0, & : T ->) : Nil + def each_step(n : Int, *, offset : Int = 0, & : T ->) : Nil offset_mod = offset % n each_with_index do |elem, i| yield elem if i >= offset && i % n == offset_mod diff --git a/src/iterable.cr b/src/iterable.cr index 73beaf97f441..ff1df82524d1 100644 --- a/src/iterable.cr +++ b/src/iterable.cr @@ -61,7 +61,7 @@ module Iterable(T) # Same as `each.skip(offset).step(n)`. # # See also: `Iterator#skip`, `Iterator#step`. - def each_step(n : Int, offset : Int) + def each_step(n : Int, *, offset : Int) each.skip(offset < 0 ? offset % n : offset).step(n) end From dd054e8aab1814d7718f39d08a64580b4acdcda4 Mon Sep 17 00:00:00 2001 From: Philip Ross Date: Fri, 30 Jun 2023 14:43:59 -0700 Subject: [PATCH 4/8] use it_iterates --- spec/std/enumerable_spec.cr | 34 +++------------------------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/spec/std/enumerable_spec.cr b/spec/std/enumerable_spec.cr index 39881383c28b..5c45fd3e2614 100644 --- a/spec/std/enumerable_spec.cr +++ b/spec/std/enumerable_spec.cr @@ -1,4 +1,5 @@ require "spec" +require "spec/helpers/iterate" private class SpecEnumerable include Enumerable(Int32) @@ -394,37 +395,8 @@ describe "Enumerable" do end describe "each_step" do - it "yields the every 2nd element" do - collection = [] of String - %w[a b c d e f].each_step(2) do |e| - collection << e - end - collection.should eq ["a", "c", "e"] - end - - it "accepts an optional offset parameter" do - collection = [] of String - %w[a b c d e f].each_step(2, offset: 1) do |e| - collection << e - end - collection.should eq ["b", "d", "f"] - end - - it "gets each_step iterator" do - iter = %w[a b c d e f].each_step(2) - iter.next.should eq("a") - iter.next.should eq("c") - iter.next.should eq("e") - iter.next.should be_a(Iterator::Stop) - end - - it "gets each_step iterator with an offset" do - iter = %w[a b c d e f].each_step(2, offset: 1) - iter.next.should eq("b") - iter.next.should eq("d") - iter.next.should eq("f") - iter.next.should be_a(Iterator::Stop) - end + it_iterates "yields the every 2nd element", %w[a c e], %w[a b c d e f].each_step(2) + it_iterates "accepts an optional offset parameter", %w[b d f], %w[a b c d e f].each_step(2, offset: 1) end describe "each_with_index" do From d98e55ab25db89e643586cf3374dba06e92f1544 Mon Sep 17 00:00:00 2001 From: Philip Ross Date: Fri, 30 Jun 2023 15:07:34 -0700 Subject: [PATCH 5/8] update docs --- src/iterable.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iterable.cr b/src/iterable.cr index ff1df82524d1..58721dc50b12 100644 --- a/src/iterable.cr +++ b/src/iterable.cr @@ -60,7 +60,7 @@ module Iterable(T) # Same as `each.skip(offset).step(n)`. # - # See also: `Iterator#skip`, `Iterator#step`. + # See also: `Iterator#step`. def each_step(n : Int, *, offset : Int) each.skip(offset < 0 ? offset % n : offset).step(n) end From c065d22d0a1d58697071a734d8ffe9859880359f Mon Sep 17 00:00:00 2001 From: Philip Ross Date: Mon, 17 Jul 2023 11:30:21 -0700 Subject: [PATCH 6/8] disallowed negative offsets and added more specs --- spec/std/enumerable_spec.cr | 24 +++++++++++++++++++++++- src/enumerable.cr | 2 +- src/iterable.cr | 2 +- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/spec/std/enumerable_spec.cr b/spec/std/enumerable_spec.cr index 5c45fd3e2614..4c3ff86b96aa 100644 --- a/spec/std/enumerable_spec.cr +++ b/spec/std/enumerable_spec.cr @@ -395,8 +395,30 @@ describe "Enumerable" do end describe "each_step" do - it_iterates "yields the every 2nd element", %w[a c e], %w[a b c d e f].each_step(2) + it_iterates "yields every 2nd element", %w[a c e], %w[a b c d e f].each_step(2) it_iterates "accepts an optional offset parameter", %w[b d f], %w[a b c d e f].each_step(2, offset: 1) + it_iterates "accepts an optional offset parameter of 0", %w[a c e], %w[a b c d e f].each_step(2, offset: 0) + + it_iterates "accepts a step larger then the enumerable size", %w[a], %w[a b c d e f].each_step(7) + it_iterates "accepts an offset larger then the enumerable size", %w[], %w[a b c d e f].each_step(1, offset: 7) + + it "doesn't accept a negative step" do + expect_raises(ArgumentError) do + %w[a b c d e f].each_step(-2) + end + end + + it "doesn't accept a step of 0" do + expect_raises(ArgumentError) do + %w[a b c d e f].each_step(0) + end + end + + it "doesn't accept a negative offset" do + expect_raises(ArgumentError) do + %w[a b c d e f].each_step(2, offset: -2) + end + end end describe "each_with_index" do diff --git a/src/enumerable.cr b/src/enumerable.cr index c4b78448af7a..05d07c84d4bb 100644 --- a/src/enumerable.cr +++ b/src/enumerable.cr @@ -444,7 +444,7 @@ module Enumerable(T) # Accepts an optional *offset* parameter # # ``` - # %w[Alice Bob Charlie David].each_step(2, 1) do |user| + # %w[Alice Bob Charlie David].each_step(2, offset: 1) do |user| # puts "User: #{user}" # end # ``` diff --git a/src/iterable.cr b/src/iterable.cr index 58721dc50b12..f2d70e85062e 100644 --- a/src/iterable.cr +++ b/src/iterable.cr @@ -62,7 +62,7 @@ module Iterable(T) # # See also: `Iterator#step`. def each_step(n : Int, *, offset : Int) - each.skip(offset < 0 ? offset % n : offset).step(n) + each.skip(offset).step(n) end # Same as `each.with_index(offset)`. From 8b9f886cf60456cda72c51edbdbfd144f0c69fd5 Mon Sep 17 00:00:00 2001 From: Philip Ross Date: Tue, 18 Jul 2023 15:07:13 -0700 Subject: [PATCH 7/8] add input validation to Enumerable#each_step and add specs. --- spec/std/enumerable_spec.cr | 12 +++++++++++- src/enumerable.cr | 2 ++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/spec/std/enumerable_spec.cr b/spec/std/enumerable_spec.cr index 4c3ff86b96aa..328ad61ba21b 100644 --- a/spec/std/enumerable_spec.cr +++ b/spec/std/enumerable_spec.cr @@ -397,7 +397,8 @@ describe "Enumerable" do describe "each_step" do it_iterates "yields every 2nd element", %w[a c e], %w[a b c d e f].each_step(2) it_iterates "accepts an optional offset parameter", %w[b d f], %w[a b c d e f].each_step(2, offset: 1) - it_iterates "accepts an optional offset parameter of 0", %w[a c e], %w[a b c d e f].each_step(2, offset: 0) + it_iterates "accepts an offset of 0", %w[a c e], %w[a b c d e f].each_step(2, offset: 0) + it_iterates "accepts an offset larger then the step size", %w[d f], %w[a b c d e f].each_step(2, offset: 3) it_iterates "accepts a step larger then the enumerable size", %w[a], %w[a b c d e f].each_step(7) it_iterates "accepts an offset larger then the enumerable size", %w[], %w[a b c d e f].each_step(1, offset: 7) @@ -406,18 +407,27 @@ describe "Enumerable" do expect_raises(ArgumentError) do %w[a b c d e f].each_step(-2) end + expect_raises(ArgumentError) do + %w[a b c d e f].each_step(-2) { } + end end it "doesn't accept a step of 0" do expect_raises(ArgumentError) do %w[a b c d e f].each_step(0) end + expect_raises(ArgumentError) do + %w[a b c d e f].each_step(0) { } + end end it "doesn't accept a negative offset" do expect_raises(ArgumentError) do %w[a b c d e f].each_step(2, offset: -2) end + expect_raises(ArgumentError) do + %w[a b c d e f].each_step(2, offset: -2) { } + end end end diff --git a/src/enumerable.cr b/src/enumerable.cr index 05d07c84d4bb..949da5aef914 100644 --- a/src/enumerable.cr +++ b/src/enumerable.cr @@ -456,6 +456,8 @@ module Enumerable(T) # User: David # ``` def each_step(n : Int, *, offset : Int = 0, & : T ->) : Nil + raise ArgumentError.new("n must be greater or equal 1") if n < 1 + raise ArgumentError.new("offset must be greater or equal 0") if offset < 0 offset_mod = offset % n each_with_index do |elem, i| yield elem if i >= offset && i % n == offset_mod From a4ddc32906f0755f0e88b6bbb5333f2c12c9c028 Mon Sep 17 00:00:00 2001 From: Philip Ross Date: Tue, 18 Jul 2023 15:55:14 -0700 Subject: [PATCH 8/8] Update src/enumerable.cr Co-authored-by: Sijawusz Pur Rahnama --- src/enumerable.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/enumerable.cr b/src/enumerable.cr index 949da5aef914..91591a4d04c7 100644 --- a/src/enumerable.cr +++ b/src/enumerable.cr @@ -456,8 +456,8 @@ module Enumerable(T) # User: David # ``` def each_step(n : Int, *, offset : Int = 0, & : T ->) : Nil - raise ArgumentError.new("n must be greater or equal 1") if n < 1 - raise ArgumentError.new("offset must be greater or equal 0") if offset < 0 + raise ArgumentError.new("Invalid n size: #{n}") if n <= 0 + raise ArgumentError.new("Invalid offset size: #{offset}") if offset < 0 offset_mod = offset % n each_with_index do |elem, i| yield elem if i >= offset && i % n == offset_mod