From 42ba3136eb5fdb918a53b13e713d4f5cae4e50e3 Mon Sep 17 00:00:00 2001 From: Prateek Singh Date: Wed, 12 Mar 2025 23:51:10 -0700 Subject: [PATCH 01/10] add find and find! methods to indexable --- spec/std/indexable_spec.cr | 44 ++++++++++++++++++++++++++++++++++++++ src/indexable.cr | 29 +++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/spec/std/indexable_spec.cr b/spec/std/indexable_spec.cr index 2d1ab60c0846..bf01b0e23f62 100644 --- a/spec/std/indexable_spec.cr +++ b/spec/std/indexable_spec.cr @@ -143,6 +143,50 @@ describe Indexable do end end + describe "#find" do + it "finds the element matching the block" do + indexable = SafeIndexable.new(4) + indexable.find { |i| i > 2 }.should eq 3 + end + + it "finds the element matching the block after given offset" do + indexable = SafeIndexable.new(8) + indexable.find(5) { |i| i.even? }.should eq 6 + end + + it "does not find the element matching the block" do + indexable = SafeIndexable.new(4) + indexable.find { |i| i > 7 }.should be_nil + end + + it "does not find the element matching the block, returns custom if_none value" do + indexable = SafeIndexable.new(4) + indexable.find(if_none: -1) { |i| i > 7 }.should eq -1 + end + + it "does not find the element matching the block after given offset, returns custom if_none value" do + indexable = SafeIndexable.new(5) + indexable.find(3, -3) { |i| i > 15 }.should eq -3 + end + end + + describe "#find!" do + it "finds the element matching the block" do + indexable = SafeIndexable.new(4) + indexable.find! { |i| i > 2 }.should eq 3 + end + + it "finds the element matching the block after given offset" do + indexable = SafeIndexable.new(8) + indexable.find!(5) { |i| i.even? }.should eq 6 + end + + it "does not find the element matching the block, raises not found" do + indexable = SafeIndexable.new(4) + expect_raises(Enumerable::NotFoundError) { indexable.find! { |i| i > 7 } } + end + end + describe "#rindex" do it "does rindex with big negative offset" do indexable = SafeIndexable.new(3) diff --git a/src/indexable.cr b/src/indexable.cr index 3f6dca1762b1..47547169929a 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -809,6 +809,35 @@ module Indexable(T) index(offset) { |e| yield e } || raise Enumerable::NotFoundError.new end + # Returns the first element in the indexable for which the passed block + # is truthy, starting from the given *offset*. + # + # Accepts an optional parameter *if_none*, to set what gets returned if + # no element is found (defaults to `nil`). + # + # ``` + # [1, 2, 3, 4].find { |i| i > 2 } # => 3 + # [1, 2, 3, 4].find(-1, 2) { |i| i < 2 } # => -1 + # [1, 2, 3, 4].find(-1) { |i| i > 8 } # => -1 + # ``` + def find(offset : Int = 0, if_none = nil, & : T ->) + offset += size if offset < 0 + return nil if offset < 0 + return (index(offset) { |i| yield i }).try { |i| unsafe_fetch(i) } || if_none + end + + # Returns the first element in the indexable for which the passed block is truthy. + # Raises `Enumerable::NotFoundError` if there is no element for which the block is truthy. + # + # ``` + # [1, 2, 3, 4].find! { |i| i > 2 } # => 3 + # [1, 2, 3, 4].find! { |i| i > 8 } # => raises Enumerable::NotFoundError + def find!(offset : Int = 0, & : T ->) + offset += size if offset < 0 + return nil if offset < 0 + return (index(offset) { |i| yield i }).try { |i| unsafe_fetch(i) } || raise Enumerable::NotFoundError.new + end + # Returns the last element of `self` if it's not empty, or raises `IndexError`. # # ``` From 48263357c62eee2d761edc2b806a072f03131310 Mon Sep 17 00:00:00 2001 From: Prateek Singh Date: Thu, 13 Mar 2025 10:47:38 -0700 Subject: [PATCH 02/10] address comment, linting --- spec/std/indexable_spec.cr | 2 +- src/indexable.cr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/std/indexable_spec.cr b/spec/std/indexable_spec.cr index bf01b0e23f62..de929ba3e848 100644 --- a/spec/std/indexable_spec.cr +++ b/spec/std/indexable_spec.cr @@ -169,7 +169,7 @@ describe Indexable do indexable.find(3, -3) { |i| i > 15 }.should eq -3 end end - + describe "#find!" do it "finds the element matching the block" do indexable = SafeIndexable.new(4) diff --git a/src/indexable.cr b/src/indexable.cr index 47547169929a..3531a105454a 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -835,7 +835,7 @@ module Indexable(T) def find!(offset : Int = 0, & : T ->) offset += size if offset < 0 return nil if offset < 0 - return (index(offset) { |i| yield i }).try { |i| unsafe_fetch(i) } || raise Enumerable::NotFoundError.new + find(offset) { |i| yield i } || raise Enumerable::NotFoundError.new end # Returns the last element of `self` if it's not empty, or raises `IndexError`. From 5d31be19c04e8f669a4cba536b83017bcc53565a Mon Sep 17 00:00:00 2001 From: Prateek Singh Date: Thu, 13 Mar 2025 14:53:38 -0700 Subject: [PATCH 03/10] remove unnecessary offset check in find! --- src/indexable.cr | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/indexable.cr b/src/indexable.cr index 3531a105454a..a9398f3c4bd5 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -833,8 +833,6 @@ module Indexable(T) # [1, 2, 3, 4].find! { |i| i > 2 } # => 3 # [1, 2, 3, 4].find! { |i| i > 8 } # => raises Enumerable::NotFoundError def find!(offset : Int = 0, & : T ->) - offset += size if offset < 0 - return nil if offset < 0 find(offset) { |i| yield i } || raise Enumerable::NotFoundError.new end From 15f406571513ef87d62fbe6e4015f8d6dbca9e02 Mon Sep 17 00:00:00 2001 From: Prateek Singh Date: Thu, 13 Mar 2025 19:51:49 -0700 Subject: [PATCH 04/10] address comments: add negative offset tests and refactor find impementation --- spec/std/indexable_spec.cr | 20 ++++++++++++++++++++ src/indexable.cr | 5 ++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/spec/std/indexable_spec.cr b/spec/std/indexable_spec.cr index de929ba3e848..b0678e7a6c48 100644 --- a/spec/std/indexable_spec.cr +++ b/spec/std/indexable_spec.cr @@ -154,6 +154,16 @@ describe Indexable do indexable.find(5) { |i| i.even? }.should eq 6 end + it "finds the element matching the block after given negative offset" do + indexable = SafeIndexable.new(8) + indexable.find(-6) { |i| i.even? }.should eq 2 + end + + it "does not receive a valid negative offset, returns nil" do + indexable = SafeIndexable.new(4) + indexable.find(-10) { |i| i > 2 }.should be_nil + end + it "does not find the element matching the block" do indexable = SafeIndexable.new(4) indexable.find { |i| i > 7 }.should be_nil @@ -181,6 +191,16 @@ describe Indexable do indexable.find!(5) { |i| i.even? }.should eq 6 end + it "finds the element matching the block after given negative offset" do + indexable = SafeIndexable.new(8) + indexable.find!(-6) { |i| i.even? }.should eq 2 + end + + it "does not receive a valid negative offset, raises not found" do + indexable = SafeIndexable.new(4) + expect_raises(Enumerable::NotFoundError) { indexable.find!(-10) { |i| i > 2 } } + end + it "does not find the element matching the block, raises not found" do indexable = SafeIndexable.new(4) expect_raises(Enumerable::NotFoundError) { indexable.find! { |i| i > 7 } } diff --git a/src/indexable.cr b/src/indexable.cr index a9398f3c4bd5..a7de0fbb212e 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -823,7 +823,10 @@ module Indexable(T) def find(offset : Int = 0, if_none = nil, & : T ->) offset += size if offset < 0 return nil if offset < 0 - return (index(offset) { |i| yield i }).try { |i| unsafe_fetch(i) } || if_none + if idx = index(offset) { |i| yield i } + return unsafe_fetch(idx) + end + if_none end # Returns the first element in the indexable for which the passed block is truthy. From 673ebce94be74e9a407852480324d70dce0225f1 Mon Sep 17 00:00:00 2001 From: Prateek Singh Date: Thu, 13 Mar 2025 20:23:40 -0700 Subject: [PATCH 05/10] address nit --- src/indexable.cr | 1 + 1 file changed, 1 insertion(+) diff --git a/src/indexable.cr b/src/indexable.cr index a7de0fbb212e..e76d98d95cec 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -823,6 +823,7 @@ module Indexable(T) def find(offset : Int = 0, if_none = nil, & : T ->) offset += size if offset < 0 return nil if offset < 0 + if idx = index(offset) { |i| yield i } return unsafe_fetch(idx) end From 6e30dcf1c856e12f3141e3b8d2b6c59f1e11a488 Mon Sep 17 00:00:00 2001 From: Prateek Singh Date: Fri, 14 Mar 2025 11:16:30 -0700 Subject: [PATCH 06/10] address comments: fix find parameter ordering --- spec/std/indexable_spec.cr | 8 ++++---- src/indexable.cr | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/std/indexable_spec.cr b/spec/std/indexable_spec.cr index b0678e7a6c48..ae0a510d5b74 100644 --- a/spec/std/indexable_spec.cr +++ b/spec/std/indexable_spec.cr @@ -151,17 +151,17 @@ describe Indexable do it "finds the element matching the block after given offset" do indexable = SafeIndexable.new(8) - indexable.find(5) { |i| i.even? }.should eq 6 + indexable.find(nil, 5) { |i| i.even? }.should eq 6 end it "finds the element matching the block after given negative offset" do indexable = SafeIndexable.new(8) - indexable.find(-6) { |i| i.even? }.should eq 2 + indexable.find(nil, -6) { |i| i.even? }.should eq 2 end it "does not receive a valid negative offset, returns nil" do indexable = SafeIndexable.new(4) - indexable.find(-10) { |i| i > 2 }.should be_nil + indexable.find(nil, -10) { |i| i > 2 }.should be_nil end it "does not find the element matching the block" do @@ -176,7 +176,7 @@ describe Indexable do it "does not find the element matching the block after given offset, returns custom if_none value" do indexable = SafeIndexable.new(5) - indexable.find(3, -3) { |i| i > 15 }.should eq -3 + indexable.find(-3, 3) { |i| i > 15 }.should eq -3 end end diff --git a/src/indexable.cr b/src/indexable.cr index e76d98d95cec..1d8b63df3434 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -820,7 +820,7 @@ module Indexable(T) # [1, 2, 3, 4].find(-1, 2) { |i| i < 2 } # => -1 # [1, 2, 3, 4].find(-1) { |i| i > 8 } # => -1 # ``` - def find(offset : Int = 0, if_none = nil, & : T ->) + def find(if_none = nil, offset : Int = 0, & : T ->) offset += size if offset < 0 return nil if offset < 0 @@ -837,7 +837,7 @@ module Indexable(T) # [1, 2, 3, 4].find! { |i| i > 2 } # => 3 # [1, 2, 3, 4].find! { |i| i > 8 } # => raises Enumerable::NotFoundError def find!(offset : Int = 0, & : T ->) - find(offset) { |i| yield i } || raise Enumerable::NotFoundError.new + find(nil, offset) { |i| yield i } || raise Enumerable::NotFoundError.new end # Returns the last element of `self` if it's not empty, or raises `IndexError`. From 023843239554407d09191641dc6cd2eaa8891cdb Mon Sep 17 00:00:00 2001 From: Prateek Singh Date: Fri, 14 Mar 2025 12:43:51 -0700 Subject: [PATCH 07/10] address comments: make use of offset parameter clear --- spec/std/indexable_spec.cr | 12 ++++++------ src/indexable.cr | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/std/indexable_spec.cr b/spec/std/indexable_spec.cr index ae0a510d5b74..ff858d318655 100644 --- a/spec/std/indexable_spec.cr +++ b/spec/std/indexable_spec.cr @@ -151,17 +151,17 @@ describe Indexable do it "finds the element matching the block after given offset" do indexable = SafeIndexable.new(8) - indexable.find(nil, 5) { |i| i.even? }.should eq 6 + indexable.find(offset: 5) { |i| i.even? }.should eq 6 end it "finds the element matching the block after given negative offset" do indexable = SafeIndexable.new(8) - indexable.find(nil, -6) { |i| i.even? }.should eq 2 + indexable.find(offset: -6) { |i| i.even? }.should eq 2 end it "does not receive a valid negative offset, returns nil" do indexable = SafeIndexable.new(4) - indexable.find(nil, -10) { |i| i > 2 }.should be_nil + indexable.find(offset: -10) { |i| i > 2 }.should be_nil end it "does not find the element matching the block" do @@ -188,17 +188,17 @@ describe Indexable do it "finds the element matching the block after given offset" do indexable = SafeIndexable.new(8) - indexable.find!(5) { |i| i.even? }.should eq 6 + indexable.find!(offset: 5) { |i| i.even? }.should eq 6 end it "finds the element matching the block after given negative offset" do indexable = SafeIndexable.new(8) - indexable.find!(-6) { |i| i.even? }.should eq 2 + indexable.find!(offset: -6) { |i| i.even? }.should eq 2 end it "does not receive a valid negative offset, raises not found" do indexable = SafeIndexable.new(4) - expect_raises(Enumerable::NotFoundError) { indexable.find!(-10) { |i| i > 2 } } + expect_raises(Enumerable::NotFoundError) { indexable.find!(offset: -10) { |i| i > 2 } } end it "does not find the element matching the block, raises not found" do diff --git a/src/indexable.cr b/src/indexable.cr index 1d8b63df3434..fef83979428e 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -837,7 +837,7 @@ module Indexable(T) # [1, 2, 3, 4].find! { |i| i > 2 } # => 3 # [1, 2, 3, 4].find! { |i| i > 8 } # => raises Enumerable::NotFoundError def find!(offset : Int = 0, & : T ->) - find(nil, offset) { |i| yield i } || raise Enumerable::NotFoundError.new + find(offset: offset) { |i| yield i } || raise Enumerable::NotFoundError.new end # Returns the last element of `self` if it's not empty, or raises `IndexError`. From 9061ff5c540ae53fb82cce2fb4db3ceff12b9c8e Mon Sep 17 00:00:00 2001 From: Prateek Singh Date: Fri, 14 Mar 2025 19:08:16 -0700 Subject: [PATCH 08/10] address comment: reorder example and add missing backticks --- src/indexable.cr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/indexable.cr b/src/indexable.cr index fef83979428e..c376ad769eed 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -817,8 +817,8 @@ module Indexable(T) # # ``` # [1, 2, 3, 4].find { |i| i > 2 } # => 3 - # [1, 2, 3, 4].find(-1, 2) { |i| i < 2 } # => -1 # [1, 2, 3, 4].find(-1) { |i| i > 8 } # => -1 + # [1, 2, 3, 4].find(-1, 2) { |i| i < 2 } # => -1 # ``` def find(if_none = nil, offset : Int = 0, & : T ->) offset += size if offset < 0 @@ -836,6 +836,7 @@ module Indexable(T) # ``` # [1, 2, 3, 4].find! { |i| i > 2 } # => 3 # [1, 2, 3, 4].find! { |i| i > 8 } # => raises Enumerable::NotFoundError + # ``` def find!(offset : Int = 0, & : T ->) find(offset: offset) { |i| yield i } || raise Enumerable::NotFoundError.new end From 95169c4e6e04882e9438b7d9afca655e14d7069a Mon Sep 17 00:00:00 2001 From: Prateek Singh Date: Sun, 16 Mar 2025 19:02:30 -0700 Subject: [PATCH 09/10] address comment: add mention of offset in find! comments --- src/indexable.cr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/indexable.cr b/src/indexable.cr index c376ad769eed..11dac8f2a101 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -830,7 +830,8 @@ module Indexable(T) if_none end - # Returns the first element in the indexable for which the passed block is truthy. + # Returns the first element in the indexable for which the passed block + # is truthy, starting from the given *offset*. # Raises `Enumerable::NotFoundError` if there is no element for which the block is truthy. # # ``` From 7dfab5cfa7c5ed155ee1ae3ada5e7d847b6c9799 Mon Sep 17 00:00:00 2001 From: Prateek Singh Date: Sun, 16 Mar 2025 19:15:06 -0700 Subject: [PATCH 10/10] address comments: added offset examples for find! --- src/indexable.cr | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/indexable.cr b/src/indexable.cr index 11dac8f2a101..ca9485a8655a 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -835,8 +835,10 @@ module Indexable(T) # Raises `Enumerable::NotFoundError` if there is no element for which the block is truthy. # # ``` - # [1, 2, 3, 4].find! { |i| i > 2 } # => 3 - # [1, 2, 3, 4].find! { |i| i > 8 } # => raises Enumerable::NotFoundError + # [1, 2, 3, 4].find! { |i| i > 2 } # => 3 + # [1, 2, 3, 4].find!(3) { |i| i > 2 } # => 4 + # [1, 2, 3, 4].find! { |i| i > 8 } # => raises Enumerable::NotFoundError + # [1, 2, 3, 4].find!(-5) { |i| i > 2 } # => raises Enumerable::NotFoundError # ``` def find!(offset : Int = 0, & : T ->) find(offset: offset) { |i| yield i } || raise Enumerable::NotFoundError.new