Skip to content

Commit 02549d9

Browse files
committed
🐛 Fix SequenceSet#max(n), cardinality < n <= size
When `SequenceSet#max(n)` is called with `n > cardinality`, it _should_ return a duplicate of the whole set. But, `#max(n)` is implemented using `#slice(-n..)`, and (copying the behavior of `Array`), when a slicing starts from an out-of-range index, it returns `nil`. It was using `-[count, size].min` to keep the index from going out-of-range. Prior to #564, `#size` was the same as `#count`, so it would give incorrect results when the set contains an endless range. After #564, this gives incorrect results when the ordered list contains duplicates. This change should also give a small performance boost, because it bypasses the complexity of `#slice(range)` and just calls `#dup` (or returns `self` when the set is frozen). This issue was one of the motivations for #563 (and #564), but then I forgot about the bug, so it wasn't fixed in time for 0.6.0!
1 parent 567518c commit 02549d9

File tree

2 files changed

+14
-1
lines changed

2 files changed

+14
-1
lines changed

lib/net/imap/sequence_set.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,11 @@ def disjoint?(other)
788788
# Related: #min, #minmax, #slice
789789
def max(count = nil, star: :*)
790790
if count
791-
slice(-[count, size].min..) || remain_frozen_empty
791+
if cardinality <= count
792+
frozen? ? self : dup
793+
else
794+
slice(-count..) || remain_frozen_empty
795+
end
792796
elsif (val = max_num)
793797
val == STAR_INT ? star : val
794798
end

test/net/imap/test_sequence_set.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,15 @@ def obj.to_sequence_set; 192_168.001_255 end
808808
assert_equal SequenceSet["678"], SequenceSet["345,678"].max(1)
809809
assert_equal SequenceSet["345,678"], SequenceSet["345,678"].max(222)
810810
assert_equal SequenceSet.empty, SequenceSet.new.max(5)
811+
# with different cardinality (150) vs size (200)
812+
set = SequenceSet["101:200,51:150"]
813+
assert_equal SequenceSet["52:200"], set.max(149)
814+
assert_equal SequenceSet["51:200"], set.max(150)
815+
assert_equal SequenceSet["51:200"], set.max(200)
816+
# with different cardinality (2**32) vs count (2**32 - 1)
817+
set = SequenceSet[1..]
818+
assert_equal SequenceSet["2:*"], set.max(2**32 - 1)
819+
assert_equal SequenceSet["1:*"], set.max(2**32)
811820
end
812821

813822
test "#minmax" do

0 commit comments

Comments
 (0)