To be sure I got this right, I started out by implementing a unit test.
To reduce the reputation of writing stuff like Merchant(discountOfTheDay: true, featured: true, disabledTemp: true)
over and over, I made a little helper method that I called makeComparator(usingPredicate:)
. It takes two integers, which are intended to be 3 bits, with each bit corresponding to one of the three boolean properties.
/// A convenience function for succint testing. Lets you write 3 digit integers to
/// intitialize merchants and compare them, rather than spelling it out like
/// `Merchant(discountOfTheDay: true, featured: true, disabledTemp: true)`
///
/// - Parameter isOrderedBefore: the predicate that would usually be passed to Array.sorted(_:)
/// - Returns: true if the merchant encoded by the first integer should come before
/// the merchant encoded by the second integer
func makeComparator(
usingPredicate isOrderedBefore: @escaping (Merchant, Merchant) -> Bool
) -> (Int, Int) -> Bool {
return { (i1: Int, i2: Int) -> Bool in
return isOrderedBefore(Merchant(fromInteger: i1), Merchant(fromInteger: i2))
}
}
Next, I'll extract your predicate out into a variable, and create my "compare" closure using my new helper function:
// Original predicate from the question
let originalPredicate = { (merchant1: Merchant, merchant2: Merchant) -> Bool in
if merchant1.discountOfDay && !merchant2.discountOfDay {
return true
}
if merchant1.featured && !merchant2.featured {
return true
}
if !merchant1.disabledTemp && merchant2.disabledTemp {
return true
}
return false
}
let compare = makeComparator(usingPredicate: originalPredicate)
I then wrote out every combination possible with 2 merchants. Each merchant has 8 possible values, so a pair of merchants has 64 values. Then, for each pairing, I manually determined which one should "come first". A pretty nice pattern emerged. I wrapped these in a unit test, and indicated where the original implementation fails:
XCTAssertEqual(compare(0b000, 0b000), false)
XCTAssertEqual(compare(0b000, 0b001), false) // Original implementation fails here
XCTAssertEqual(compare(0b000, 0b010), false)
XCTAssertEqual(compare(0b000, 0b011), false) // Original implementation fails here
XCTAssertEqual(compare(0b000, 0b100), false)
XCTAssertEqual(compare(0b000, 0b101), false) // Original implementation fails here
XCTAssertEqual(compare(0b000, 0b110), false)
XCTAssertEqual(compare(0b000, 0b111), false) // Original implementation fails here
XCTAssertEqual(compare(0b001, 0b000), true) // Original implementation fails here
XCTAssertEqual(compare(0b001, 0b001), false)
XCTAssertEqual(compare(0b001, 0b010), false)
XCTAssertEqual(compare(0b001, 0b011), false)
XCTAssertEqual(compare(0b001, 0b100), false)
XCTAssertEqual(compare(0b001, 0b101), false)
XCTAssertEqual(compare(0b001, 0b110), false)
XCTAssertEqual(compare(0b001, 0b111), false)
XCTAssertEqual(compare(0b010, 0b000), true)
XCTAssertEqual(compare(0b010, 0b001), true)
XCTAssertEqual(compare(0b010, 0b010), false)
XCTAssertEqual(compare(0b010, 0b011), false) // Original implementation fails here
XCTAssertEqual(compare(0b010, 0b100), false) // Original implementation fails here
XCTAssertEqual(compare(0b010, 0b101), false) // Original implementation fails here
XCTAssertEqual(compare(0b010, 0b110), false)
XCTAssertEqual(compare(0b010, 0b111), false) // Original implementation fails here
XCTAssertEqual(compare(0b011, 0b000), true)
XCTAssertEqual(compare(0b011, 0b001), true)
XCTAssertEqual(compare(0b011, 0b010), true) // Original implementation fails here
XCTAssertEqual(compare(0b011, 0b011), false)
XCTAssertEqual(compare(0b011, 0b100), false) // Original implementation fails here
XCTAssertEqual(compare(0b011, 0b101), false) // Original implementation fails here
XCTAssertEqual(compare(0b011, 0b110), false)
XCTAssertEqual(compare(0b011, 0b111), false)
XCTAssertEqual(compare(0b100, 0b000), true)
XCTAssertEqual(compare(0b100, 0b001), true)
XCTAssertEqual(compare(0b100, 0b010), true)
XCTAssertEqual(compare(0b100, 0b011), true)
XCTAssertEqual(compare(0b100, 0b100), false)
XCTAssertEqual(compare(0b100, 0b101), false) // Original implementation fails here
XCTAssertEqual(compare(0b100, 0b110), false)
XCTAssertEqual(compare(0b100, 0b111), false) // Original implementation fails here
XCTAssertEqual(compare(0b101, 0b000), true)
XCTAssertEqual(compare(0b101, 0b001), true)
XCTAssertEqual(compare(0b101, 0b010), true)
XCTAssertEqual(compare(0b101, 0b011), true)
XCTAssertEqual(compare(0b101, 0b100), true) // Original implementation fails here
XCTAssertEqual(compare(0b101, 0b101), false)
XCTAssertEqual(compare(0b101, 0b110), false)
XCTAssertEqual(compare(0b101, 0b111), false)
XCTAssertEqual(compare(0b110, 0b000), true)
XCTAssertEqual(compare(0b110, 0b001), true)
XCTAssertEqual(compare(0b110, 0b010), true)
XCTAssertEqual(compare(0b110, 0b011), true)
XCTAssertEqual(compare(0b110, 0b100), true)
XCTAssertEqual(compare(0b110, 0b101), true)
XCTAssertEqual(compare(0b110, 0b110), false)
XCTAssertEqual(compare(0b110, 0b111), false) // Original implementation fails here
XCTAssertEqual(compare(0b111, 0b000), true)
XCTAssertEqual(compare(0b111, 0b001), true)
XCTAssertEqual(compare(0b111, 0b010), true)
XCTAssertEqual(compare(0b111, 0b011), true)
XCTAssertEqual(compare(0b111, 0b100), true)
XCTAssertEqual(compare(0b111, 0b101), true)
XCTAssertEqual(compare(0b111, 0b110), true) // Original implementation fails here
XCTAssertEqual(compare(0b111, 0b111), false)
I propose a different way to implement it, along the lines of this other answer of mine.
For every property, check if the two merchants have the same value. If they're different, compare the two properties and return true for whichever merchant should come first. Like so:
extension Bool {
enum BoolSortOrdering { case trueComesFirst, falseComesFrist }
func areInIncreasingOrder(comparedTo other: Bool, soThat ordering: BoolSortOrdering) -> Bool {
switch ordering {
case .trueComesFirst: return self && !other
case .falseComesFrist: return !self && other
}
}
}
let fixedPrecidate = { (m1: Merchant, m2: Merchant) -> Bool in
if m1.discountOfDay != m2.discountOfDay {
return m1.discountOfDay.areInIncreasingOrder(comparedTo: m2.discountOfDay, soThat: .trueComesFirst)
}
if m1.featured != m2.featured {
return m1.featured.areInIncreasingOrder(comparedTo: m2.featured, soThat: .trueComesFirst)
}
if m1.disabledTemp != m2.disabledTemp {
return m1.disabledTemp.areInIncreasingOrder(comparedTo: m2.disabledTemp, soThat: .trueComesFirst)
}
return false // the instances compare equally.
}
Here's my complete unit test file, which you can run for yourself:
struct Merchant {
let discountOfDay: Bool
let featured: Bool
let disabledTemp: Bool
}
extension Merchant {
init(fromInteger i: Int) {
self.init(
discountOfDay: i & 0b100 == 0b100,
featured: i & 0b010 == 0b010,
disabledTemp: i & 0b001 == 0b001)
}
}
// Original predicate from the question
let originalPredicate = { (merchant1: Merchant, merchant2: Merchant) -> Bool in
if merchant1.discountOfDay && !merchant2.discountOfDay {
return true
}
if merchant1.featured && !merchant2.featured {
return true
}
if !merchant1.disabledTemp && merchant2.disabledTemp {
return true
}
return false
}
extension Bool {
enum BoolSortOrdering { case trueComesFirst, falseComesFrist }
func areInIncreasingOrder(comparedTo other: Bool, soThat ordering: BoolSortOrdering) -> Bool {
switch ordering {
case .trueComesFirst: return self && !other
case .falseComesFrist: return !self && other
}
}
}
let fixedPrecidate = { (m1: Merchant, m2: Merchant) -> Bool in
if m1.discountOfDay != m2.discountOfDay {
return m1.discountOfDay.areInIncreasingOrder(comparedTo: m2.discountOfDay, soThat: .trueComesFirst)
}
if m1.featured != m2.featured {
return m1.featured.areInIncreasingOrder(comparedTo: m2.featured, soThat: .trueComesFirst)
}
if m1.disabledTemp != m2.disabledTemp {
return m1.disabledTemp.areInIncreasingOrder(comparedTo: m2.disabledTemp, soThat: .trueComesFirst)
}
return false // the instances compare equally.
}
class MerchantTests: XCTestCase {
func testComparison() {
/// A convenience function for succint testing. Lets you write 3 digit integers to
/// intitialize merchants and compare them, rather than spelling it out like
/// `Merchant(discountOfTheDay: true, featured: true, disabledTemp: true)`
///
/// - Parameter isOrderedBefore: the predicate that would usually be passed to Array.sorted(_:)
/// - Returns: true if the merchant encoded by the first integer should come before
/// the merchant encoded by the second integer
func makeComparator(
usingPredicate isOrderedBefore: @escaping (Merchant, Merchant) -> Bool
) -> (Int, Int) -> Bool {
return { (i1: Int, i2: Int) -> Bool in
return isOrderedBefore(Merchant(fromInteger: i1), Merchant(fromInteger: i2))
}
}
let compare = makeComparator(usingPredicate: fixedPrecidate)
// let compare = makeComparator(usingPredicate: originalPredicate)
XCTAssertEqual(compare(0b000, 0b000), false)
XCTAssertEqual(compare(0b000, 0b001), false) // Original implementation fails here
XCTAssertEqual(compare(0b000, 0b010), false)
XCTAssertEqual(compare(0b000, 0b011), false) // Original implementation fails here
XCTAssertEqual(compare(0b000, 0b100), false)
XCTAssertEqual(compare(0b000, 0b101), false) // Original implementation fails here
XCTAssertEqual(compare(0b000, 0b110), false)
XCTAssertEqual(compare(0b000, 0b111), false) // Original implementation fails here
XCTAssertEqual(compare(0b001, 0b000), true) // Original implementation fails here
XCTAssertEqual(compare(0b001, 0b001), false)
XCTAssertEqual(compare(0b001, 0b010), false)
XCTAssertEqual(compare(0b001, 0b011), false)
XCTAssertEqual(compare(0b001, 0b100), false)
XCTAssertEqual(compare(0b001, 0b101), false)
XCTAssertEqual(compare(0b001, 0b110), false)
XCTAssertEqual(compare(0b001, 0b111), false)
XCTAssertEqual(compare(0b010, 0b000), true)
XCTAssertEqual(compare(0b010, 0b001), true)
XCTAssertEqual(compare(0b010, 0b010), false)
XCTAssertEqual(compare(0b010, 0b011), false) // Original implementation fails here
XCTAssertEqual(compare(0b010, 0b100), false) // Original implementation fails here
XCTAssertEqual(compare(0b010, 0b101), false) // Original implementation fails here
XCTAssertEqual(compare(0b010, 0b110), false)
XCTAssertEqual(compare(0b010, 0b111), false) // Original implementation fails here
XCTAssertEqual(compare(0b011, 0b000), true)
XCTAssertEqual(compare(0b011, 0b001), true)
XCTAssertEqual(compare(0b011, 0b010), true) // Original implementation fails here
XCTAssertEqual(compare(0b011, 0b011), false)
XCTAssertEqual(compare(0b011, 0b100), false) // Original implementation fails here
XCTAssertEqual(compare(0b011, 0b101), false) // Original implementation fails here
XCTAssertEqual(compare(0b011, 0b110), false)
XCTAssertEqual(compare(0b011, 0b111), false)
XCTAssertEqual(compare(0b100, 0b000), true)
XCTAssertEqual(compare(0b100, 0b001), true)
XCTAssertEqual(compare(0b100, 0b010), true)
XCTAssertEqual(compare(0b100, 0b011), true)
XCTAssertEqual(compare(0b100, 0b100), false)
XCTAssertEqual(compare(0b100, 0b101), false) // Original implementation fails here
XCTAssertEqual(compare(0b100, 0b110), false)
XCTAssertEqual(compare(0b100, 0b111), false) // Original implementation fails here
XCTAssertEqual(compare(0b101, 0b000), true)
XCTAssertEqual(compare(0b101, 0b001), true)
XCTAssertEqual(compare(0b101, 0b010), true)
XCTAssertEqual(compare(0b101, 0b011), true)
XCTAssertEqual(compare(0b101, 0b100), true) // Original implementation fails here
XCTAssertEqual(compare(0b101, 0b101), false)
XCTAssertEqual(compare(0b101, 0b110), false)
XCTAssertEqual(compare(0b101, 0b111), false)
XCTAssertEqual(compare(0b110, 0b000), true)
XCTAssertEqual(compare(0b110, 0b001), true)
XCTAssertEqual(compare(0b110, 0b010), true)
XCTAssertEqual(compare(0b110, 0b011), true)
XCTAssertEqual(compare(0b110, 0b100), true)
XCTAssertEqual(compare(0b110, 0b101), true)
XCTAssertEqual(compare(0b110, 0b110), false)
XCTAssertEqual(compare(0b110, 0b111), false) // Original implementation fails here
XCTAssertEqual(compare(0b111, 0b000), true)
XCTAssertEqual(compare(0b111, 0b001), true)
XCTAssertEqual(compare(0b111, 0b010), true)
XCTAssertEqual(compare(0b111, 0b011), true)
XCTAssertEqual(compare(0b111, 0b100), true)
XCTAssertEqual(compare(0b111, 0b101), true)
XCTAssertEqual(compare(0b111, 0b110), true) // Original implementation fails here
XCTAssertEqual(compare(0b111, 0b111), false)
}
}