From 39859999e0bfc9c0e257c2738e4bc9ae406610fa Mon Sep 17 00:00:00 2001 From: Carlos Gajardo Date: Thu, 6 Oct 2016 22:32:10 -0300 Subject: [PATCH] Implement no-copy subarray iteration --- spec/std/indexable_spec.cr | 64 +++++++++++++++++++++++++- src/array.cr | 14 ------ src/indexable.cr | 94 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 15 deletions(-) diff --git a/spec/std/indexable_spec.cr b/spec/std/indexable_spec.cr index ed49e6d476ae..f7ca0fbc1541 100644 --- a/spec/std/indexable_spec.cr +++ b/spec/std/indexable_spec.cr @@ -3,7 +3,7 @@ require "spec" class SafeIndexable include Indexable(Int32) - getter size + property size def initialize(@size : Int32) end @@ -34,4 +34,66 @@ describe Indexable do indexable = SafeIndexable.new(3) indexable.rindex(0, 100).should be_nil end + + it "iterates throught a subset of its elements (#3386)" do + indexable = SafeIndexable.new(5) + last_element = nil + + return_value = indexable.each(start: 2, count: 3) do |elem| + last_element = elem + end + + return_value.should eq(indexable) + last_element.should eq(4) + end + + it "iterates until its size (#3386)" do + indexable = SafeIndexable.new(5) + last_element = nil + + indexable.each(start: 3, count: 999) do |elem| + last_element = elem + end + + last_element.should eq(4) + end + + it "iterates until its size, having mutated (#3386)" do + indexable = SafeIndexable.new(10) + last_element = nil + + indexable.each(start: 3, count: 999) do |elem| + indexable.size += 1 if elem <= 5 + # size is incremented 3 times + last_element = elem + end + + # last was 9, but now is 12. + last_element.should eq(12) + end + + it "iterates until its size, having mutated (#3386)" do + indexable = SafeIndexable.new(10) + last_element = nil + + indexable.each(start: 3, count: 5) do |elem| + indexable.size += 1 + last_element = elem + end + + # last element iterated is still 7. + last_element.should eq(7) + end + + it "iterates within a range of indices (#3386)" do + indexable = SafeIndexable.new(5) + last_element = nil + + return_value = indexable.each(within: 2..3) do |elem| + last_element = elem + end + + return_value.should eq(indexable) + last_element.should eq(3) + end end diff --git a/src/array.cr b/src/array.cr index 333811f36c16..5c160fe057f9 100644 --- a/src/array.cr +++ b/src/array.cr @@ -1774,20 +1774,6 @@ class Array(T) end end - private def range_to_index_and_count(range) - from = range.begin - from += size if from < 0 - raise IndexError.new if from < 0 - - to = range.end - to += size if to < 0 - to -= 1 if range.excludes_end? - size = to - from + 1 - size = 0 if size < 0 - - {from, size} - end - private class PermutationIterator(T) include Iterator(Array(T)) diff --git a/src/indexable.cr b/src/indexable.cr index eb877efc65ae..b58ecbdfe309 100644 --- a/src/indexable.cr +++ b/src/indexable.cr @@ -168,6 +168,51 @@ module Indexable(T) ItemIterator(self, T).new(self) end + # Calls the given block once for `count` number of elements in `self` + # starting from index `start`, passing each element as a parameter. + # + # Negative indices count backward from the end of the array. (-1 is the + # last element). + # + # Raises `IndexError` if the starting index is out of range. + # Raises `ArgumentError` if `count` is a negative number. + # + # ``` + # array = ["a", "b", "c", "d", "e"] + # array.each(start: 1, count: 3) { |x| print x, " -- " } + # ``` + # + # produces: + # + # ```text + # b -- c -- d -- + # ``` + def each(*, start : Int, count : Int) + each_index(start: start, count: count) do |i| + yield unsafe_at(i) + end + end + + # Calls the given block once for all elements at indices within the given + # `range`, passing each element as a parameter. + # + # Raises `IndexError` if the starting index is out of range. + # + # ``` + # array = ["a", "b", "c", "d", "e"] + # array.each(1..3) { |x| print x, " -- " } + # ``` + # + # produces: + # + # ```text + # b -- c -- d -- + # ``` + def each(*, within range : Range(Int, Int)) + start, count = range_to_index_and_count(range) + each(start: start, count: count) { |element| yield element } + end + # Calls the given block once for each index in `self`, passing that # index as a parameter. # @@ -205,6 +250,41 @@ module Indexable(T) IndexIterator.new(self) end + # Calls the given block once for `count` number of indices in `self` + # starting from index `start`, passing each index as a parameter. + # + # Negative indices count backward from the end of the array. (-1 is the + # last element). + # + # Raises `IndexError` if the starting index is out of range. + # Raises `ArgumentError` if `count` is a negative number. + # + # ``` + # array = ["a", "b", "c", "d", "e"] + # array.each(start: 1, count: 3) { |x| print x, " -- " } + # ``` + # + # produces: + # + # ```text + # 1 -- 2 -- 3 -- + # ``` + def each_index(*, start : Int, count : Int) + raise ArgumentError.new "negative count: #{count}" if count < 0 + + start += size if start < 0 + raise IndexError.new unless 0 <= start <= size + + i = start + # `count` and size comparison must be done every iteration because + # `self` can mutate in the block. + while i < Math.min(start + count, size) + yield i + i += 1 + end + self + end + # Returns *true* if `self` is empty, *false* otherwise. # # ``` @@ -421,6 +501,20 @@ module Indexable(T) end end + private def range_to_index_and_count(range) + from = range.begin + from += size if from < 0 + raise IndexError.new if from < 0 + + to = range.end + to += size if to < 0 + to -= 1 if range.excludes_end? + size = to - from + 1 + size = 0 if size < 0 + + {from, size} + end + private class ItemIterator(A, T) include Iterator(T)