mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Improve String#first and #last performance
Removes unnecessary conditional and method call for significant performance improvement. As a side effect, this fixes an unexpected behavior where passing a limit of 0 would return a frozen string. This also implements the Rails 6.1 intended behavior with regards to negative limits, and removes the previous deprecation warnings. String#first Comparison: new: 3056515.0 i/s old: 1943310.2 i/s - 1.57x slower String#last Comparison: new: 2691919.0 i/s old: 1924256.6 i/s - 1.40x slower (Note: "old" benchmarks have deprecation warnings commented out, for a more fair comparison.)
This commit is contained in:
parent
9b0e632def
commit
0e2de0e3fd
2 changed files with 22 additions and 34 deletions
|
@ -75,17 +75,7 @@ class String
|
|||
# str.first(0) # => ""
|
||||
# str.first(6) # => "hello"
|
||||
def first(limit = 1)
|
||||
ActiveSupport::Deprecation.warn(
|
||||
"Calling String#first with a negative integer limit " \
|
||||
"will raise an ArgumentError in Rails 6.1."
|
||||
) if limit < 0
|
||||
if limit == 0
|
||||
""
|
||||
elsif limit >= size
|
||||
dup
|
||||
else
|
||||
to(limit - 1)
|
||||
end
|
||||
self[0, limit] || (raise ArgumentError, "negative limit")
|
||||
end
|
||||
|
||||
# Returns the last character of the string. If a limit is supplied, returns a substring
|
||||
|
@ -99,16 +89,6 @@ class String
|
|||
# str.last(0) # => ""
|
||||
# str.last(6) # => "hello"
|
||||
def last(limit = 1)
|
||||
ActiveSupport::Deprecation.warn(
|
||||
"Calling String#last with a negative integer limit " \
|
||||
"will raise an ArgumentError in Rails 6.1."
|
||||
) if limit < 0
|
||||
if limit == 0
|
||||
""
|
||||
elsif limit >= size
|
||||
dup
|
||||
else
|
||||
from(-limit)
|
||||
end
|
||||
self[[length - limit, 0].max, limit] || (raise ArgumentError, "negative limit")
|
||||
end
|
||||
end
|
||||
|
|
|
@ -480,12 +480,16 @@ class StringAccessTest < ActiveSupport::TestCase
|
|||
assert_not_same different_string, string
|
||||
end
|
||||
|
||||
test "#first with negative Integer is deprecated" do
|
||||
string = "hello"
|
||||
message = "Calling String#first with a negative integer limit " \
|
||||
"will raise an ArgumentError in Rails 6.1."
|
||||
assert_deprecated(message) do
|
||||
string.first(-1)
|
||||
test "#first with Integer returns a non-frozen string" do
|
||||
string = "he"
|
||||
(0..string.length + 1).each do |limit|
|
||||
assert_not string.first(limit).frozen?
|
||||
end
|
||||
end
|
||||
|
||||
test "#first with negative Integer raises ArgumentError" do
|
||||
assert_raise ArgumentError do
|
||||
"hello".first(-1)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -507,12 +511,16 @@ class StringAccessTest < ActiveSupport::TestCase
|
|||
assert_not_same different_string, string
|
||||
end
|
||||
|
||||
test "#last with negative Integer is deprecated" do
|
||||
string = "hello"
|
||||
message = "Calling String#last with a negative integer limit " \
|
||||
"will raise an ArgumentError in Rails 6.1."
|
||||
assert_deprecated(message) do
|
||||
string.last(-1)
|
||||
test "#last with Integer returns a non-frozen string" do
|
||||
string = "he"
|
||||
(0..string.length + 1).each do |limit|
|
||||
assert_not string.last(limit).frozen?
|
||||
end
|
||||
end
|
||||
|
||||
test "#last with negative Integer raises ArgumentError" do
|
||||
assert_raise ArgumentError do
|
||||
"hello".last(-1)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in a new issue