Skip to content

Commit

Permalink
Fix String#compare's behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
r00ster91 committed Feb 2, 2019
1 parent 939d0e5 commit 1ec41f9
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 38 deletions.
39 changes: 23 additions & 16 deletions spec/std/string_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2178,22 +2178,29 @@ describe "String" do
end
end

it "compares non-case insensitive" do
"fo".compare("foo").should eq(-1)
"foo".compare("fo").should eq(1)
"foo".compare("foo").should eq(0)
"foo".compare("fox").should eq(-1)
"fox".compare("foo").should eq(1)
"foo".compare("Foo").should eq(1)
end

it "compares case insensitive" do
"fo".compare("FOO", case_insensitive: true).should eq(-1)
"foo".compare("FO", case_insensitive: true).should eq(1)
"foo".compare("FOO", case_insensitive: true).should eq(0)
"foo".compare("FOX", case_insensitive: true).should eq(-1)
"fox".compare("FOO", case_insensitive: true).should eq(1)
"fo\u{0000}".compare("FO", case_insensitive: true).should eq(1)
describe "compare" do
it "compares non-case-insensitive" do
"fo".compare("foo").should eq(-1)
"foo".compare("fo").should eq(1)
"foo".compare("foo").should eq(0)
"foo".compare("fox").should eq(-1)
"fox".compare("foo").should eq(1)
"foo".compare("Foo").should eq(1)
end

it "compares case-insensitive" do
"FOX".compare("foo", case_insensitive: true).should eq(-1)
"foo".compare("FOX", case_insensitive: true).should eq(1)
"fo".compare("FOO", case_insensitive: true).should eq(1)
"foo".compare("FO", case_insensitive: true).should eq(1)
"foo".compare("FOO", case_insensitive: true).should eq(0)
"hELLo".compare("HellO", case_insensitive: true).should eq(0)
"fo\u{0}".compare("FO", case_insensitive: true).should eq(1)
"z".compare("hello", case_insensitive: true).should eq(1)
"h".compare("zzz", case_insensitive: true).should eq(-1)
"ä".compare("äA", case_insensitive: true).should eq(-1)
"äA".compare("ä", case_insensitive: true).should eq(1)
end
end

it "builds with write_byte" do
Expand Down
39 changes: 17 additions & 22 deletions src/string.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2389,9 +2389,6 @@ class String
# this string is less, equal or greater than *other*, optionally in a *case_insensitive*
# manner.
#
# If *case_insensitive* is `false`, this method delegates to `<=>`. Otherwise,
# the strings are compared char-by-char.
#
# ```
# "abcdef".compare("abcde") # => 1
# "abcdef".compare("abcdef") # => 0
Expand All @@ -2402,30 +2399,28 @@ class String
# "abcdef".compare("ABCDEG", case_insensitive: true) # => -1
# ```
def compare(other : String, case_insensitive = false)
return self <=> other unless case_insensitive
comparison = self <=> other
return comparison if (!case_insensitive || comparison != 0) && self.bytesize != other.bytesize

if ascii_only? && other.ascii_only?
if self.size > other.size
return 1
elsif self.size < other.size
return -1
end
@bytesize.times do |i|
char1 = to_unsafe[i]
char2 = other.to_unsafe[i]
byte1 = byte1_copy = to_unsafe[i]
byte2 = byte2_copy = other.to_unsafe[i]

# Lowercase both chars
if 65 <= char1 <= 90
char1 += 32
# Lowercase both bytes
if 65 <= byte1 <= 90
byte1 += 32
end
if 65 <= char2 <= 90
char2 += 32
if 65 <= byte2 <= 90
byte2 += 32
end

if char1 > char2
return 1
elsif char1 < char2
return -1
if byte1 != byte2
if byte1_copy > byte2_copy
return 1
else
return -1
end
end
end
0
Expand All @@ -2436,8 +2431,8 @@ class String
char2 = reader2.current_char

while reader1.has_next? && reader2.has_next?
cmp = char1.downcase <=> char2.downcase
return cmp.sign if cmp != 0
comparison = char1.downcase <=> char2.downcase
return comparison.sign if comparison != 0

char1 = reader1.next_char
char2 = reader2.next_char
Expand Down

0 comments on commit 1ec41f9

Please sign in to comment.