0

I'm trying to sort array based on 3 boolean variables if they are not adjacent it's working fine but after 1st sort, if same method called it's not working as expected. Following is code i'm using.

I need to sort array such as Discount of day should be on top, then featured and all disabled must be at bottom.

data.dataSource.sort { (merchant1, merchant2) -> 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
    }
6
  • 1
    This is a little tricky. I recommend you implement this by starting with unit tests. There are 8 possible values of a merchant. Consider the notation "xxx", where each x is 0 or 1. The first position is the value of discountOfDay, the second is featured, and the third is disabledTemp. There are 8 possible values of a merchant: 000, 001, 010, 011, 100, 101, 110, 111. Now consider the possible values of 2 merchants: it's every possible pairing of those two: (000, 000), (000, 001), ..., (000, 111), ..., (111, 000), ... (111, 111). Commented Sep 1, 2019 at 19:40
  • 1
    Once you have these 64 pairs of two merchants, you can write one XCTAssertEqual line for each one, and assert that your comparison function's output matches the expected value (which you have to come up with yourself). Once you have these unit tests, they basically make a spec that defines when you've correctly implemented this. From there you can start tweaking your comparison function Commented Sep 1, 2019 at 19:41
  • 3
    Replace the 3 booleans by a single integer (or better an enum with raw integer value). Then you can simply compare with < Commented Sep 1, 2019 at 19:42
  • I forgot to ask if any valid combination of these 3 booleans is possible. I assumed that was the case. If they're mutually exclusive, you should use an enum, as martin mentioned. Commented Sep 1, 2019 at 19:44
  • 1
    @Adrian Oh look, that's me! :) Commented Sep 1, 2019 at 20:40

1 Answer 1

2

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)

    }
}
Sign up to request clarification or add additional context in comments.

4 Comments

thank you alexander for detailed answer, i will look into briefly and implement as you have explained
You didn't answer my question in my comment on your original post. Are these boolean flags mutually exclusive?
featured and DOD are mutually exclusive, disabled can come with anyone, it could be with featured or discount of day
@ArslanAsim Then you should make Featured and DOD into an enum, along with a "neither" case.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.