Skip to content

Fix NSArray.indexOfObject(inSortedRange:options:usingComparator) for fringe cases #130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Dec 18, 2015
Merged
34 changes: 26 additions & 8 deletions Foundation/NSArray.swift
Original file line number Diff line number Diff line change
Expand Up @@ -412,22 +412,44 @@ public class NSArray : NSObject, NSCopying, NSMutableCopying, NSSecureCoding, NS
}

public func indexOfObject(obj: AnyObject, inSortedRange r: NSRange, options opts: NSBinarySearchingOptions, usingComparator cmp: NSComparator) -> Int {
guard (r.location + r.length) <= count else {
NSInvalidArgument("range \(r) extends beyond bounds [0 .. \(count - 1)]")
let lastIndex = r.location + r.length - 1

// argument validation
guard lastIndex < count else {
let bounds = count == 0 ? "for empty array" : "[0 .. \(count - 1)]"
NSInvalidArgument("range \(r) extends beyond bounds \(bounds)")
}

if opts.contains(.FirstEqual) && opts.contains(.LastEqual) {
NSInvalidArgument("both NSBinarySearching.FirstEqual and NSBinarySearching.LastEqual options cannot be specified")
}

let searchForInsertionIndex = opts.contains(.InsertionIndex)

// fringe cases
if r.length == 0 {
return searchForInsertionIndex ? r.location : NSNotFound
}

let leastObj = objectAtIndex(r.location)
if cmp(obj, leastObj) == .OrderedAscending {
return searchForInsertionIndex ? r.location : NSNotFound
}

let greatestObj = objectAtIndex(lastIndex)
if cmp(obj, greatestObj) == .OrderedDescending {
return searchForInsertionIndex ? lastIndex + 1 : NSNotFound
}

// common processing
let firstEqual = opts.contains(.FirstEqual)
let lastEqual = opts.contains(.LastEqual)
let anyEqual = !(firstEqual || lastEqual)

var result = NSNotFound
var indexOfLeastGreaterThanObj = NSNotFound
var start = r.location
var end = r.location + r.length - 1
var end = lastIndex

loop: while start <= end {
let middle = start + (end - start) / 2
Expand Down Expand Up @@ -459,18 +481,14 @@ public class NSArray : NSObject, NSCopying, NSMutableCopying, NSSecureCoding, NS
}
}

guard opts.contains(.InsertionIndex) && lastEqual else {
guard searchForInsertionIndex && lastEqual else {
return result
}

guard result == NSNotFound else {
return result + 1
}

guard indexOfLeastGreaterThanObj != NSNotFound else {
return count
}

return indexOfLeastGreaterThanObj
}

Expand Down
2 changes: 1 addition & 1 deletion Foundation/NSObjCRuntime.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public let NSNotFound: Int = Int.max
fatalError("\(fn) is not yet implemented")
}

@noreturn func NSInvalidArgument(message: String, method: String = __FUNCTION__) {
@noreturn internal func NSInvalidArgument(message: String, method: String = __FUNCTION__) {
fatalError("\(method): \(message)")
}

Expand Down
39 changes: 37 additions & 2 deletions TestFoundation/TestNSArray.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class TestNSArray : XCTestCase {
("test_getObjects", test_getObjects),
("test_objectAtIndex", test_objectAtIndex),
("test_binarySearch", test_binarySearch),
("test_binarySearchFringeCases", test_binarySearchFringeCases),
("test_replaceObjectsInRange_withObjectsFromArray", test_replaceObjectsInRange_withObjectsFromArray),
("test_replaceObjectsInRange_withObjectsFromArray_range", test_replaceObjectsInRange_withObjectsFromArray_range),
]
Expand Down Expand Up @@ -140,8 +141,42 @@ class TestNSArray : XCTestCase {
let indexOfLeastGreaterObjectThanFive = objectIndexInArray(array, value: 5, startingFrom: 0, length: 10, options: [.InsertionIndex, .LastEqual])
XCTAssertTrue(indexOfLeastGreaterObjectThanFive == 7, "If both .InsertionIndex and .LastEqual are specified NSArray returns the index of the least greater object...")

let endOfArray = objectIndexInArray(array, value: 10, startingFrom: 0, length: 13, options: [.InsertionIndex, .LastEqual])
XCTAssertTrue(endOfArray == array.count, "...or the index at the end of the array if the object is larger than all other elements.")
let rangeStart = 0
let rangeLength = 13
let endOfArray = objectIndexInArray(array, value: 10, startingFrom: rangeStart, length: rangeLength, options: [.InsertionIndex, .LastEqual])
XCTAssertTrue(endOfArray == (rangeStart + rangeLength), "...or the index at the end of the array if the object is larger than all other elements.")
}

func test_binarySearchFringeCases() {
let array = NSArray(array: [
NSNumber(int: 0), NSNumber(int: 1), NSNumber(int: 2), NSNumber(int: 2), NSNumber(int: 3),
NSNumber(int: 4), NSNumber(int: 4), NSNumber(int: 6), NSNumber(int: 7), NSNumber(int: 7),
NSNumber(int: 7), NSNumber(int: 8), NSNumber(int: 9), NSNumber(int: 9)])

let emptyArray = NSArray()
// Same as for non empty NSArray but error message ends with 'bounds for empty array'.
// let _ = objectIndexInArray(emptyArray, value: 0, startingFrom: 0, length: 1)

let notFoundInEmptyArray = objectIndexInArray(emptyArray, value: 9, startingFrom: 0, length: 0)
XCTAssertEqual(notFoundInEmptyArray, NSNotFound, "Empty NSArray return NSNotFound for any valid arguments.")

let startIndex = objectIndexInArray(emptyArray, value: 7, startingFrom: 0, length: 0, options: [.InsertionIndex])
XCTAssertTrue(startIndex == 0, "For Empty NSArray any objects should be inserted at start.")

let rangeStart = 0
let rangeLength = 13

let leastSearch = objectIndexInArray(array, value: -1, startingFrom: rangeStart, length: rangeLength)
XCTAssertTrue(leastSearch == NSNotFound, "If object is less than least object in the range then there is no change it could be found.")

let greatestSearch = objectIndexInArray(array, value: 15, startingFrom: rangeStart, length: rangeLength)
XCTAssertTrue(greatestSearch == NSNotFound, "If object is greater than greatest object in the range then there is no change it could be found.")

let leastInsert = objectIndexInArray(array, value: -1, startingFrom: rangeStart, length: rangeLength, options: .InsertionIndex)
XCTAssertTrue(leastInsert == rangeStart, "If object is less than least object in the range it should be inserted at range' location.")

let greatestInsert = objectIndexInArray(array, value: 15, startingFrom: rangeStart, length: rangeLength, options: .InsertionIndex)
XCTAssertTrue(greatestInsert == (rangeStart + rangeLength), "If object is greater than greatest object in the range it should be inserted at range' end.")
}

func objectIndexInArray(array: NSArray, value: Int, startingFrom: Int, length: Int, options: NSBinarySearchingOptions = []) -> Int {
Expand Down