Skip to content
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

Sort MutableObservableArray<Item> #376

Closed
iandundas opened this issue Jan 25, 2017 · 13 comments
Closed

Sort MutableObservableArray<Item> #376

iandundas opened this issue Jan 25, 2017 · 13 comments
Labels

Comments

@iandundas
Copy link
Contributor

In ReactiveKit 2 there was the rather magical ability to sort a CollectionProperty<[Item]>'s events in-flight, as in:

let collection: CollectionProperty<[Item]>

let sorted: Stream<CollectionChangeset<[Item]>> = collection.sort { (a: Item, b:Item) -> Bool in 
    return a.whatever > b.whatever
}

Similar to #335, could it be possible to add this to the roadmap for Bond (6?), to port this functionality over to the new MutableObservableArray type?

Cheers!

@iandundas
Copy link
Contributor Author

The hard bit is mapping the ObservableArrayChange:

let sorted = collection.map { (event: ObservableArrayEvent<String>) -> ObservableArrayEvent<String> in
  
    event.dataSource.sorted(by: (String, String) -> Bool) // easy to sort

    // The hard bit: 
    switch event.change{
    case .inserts([Int]):break;
    case .deletes([Int]):break;
    case .move(Int, Int):break;
    case .updates([Int]): break;
    } 
  }

Taking a look at the algorithm used in ReactiveKit 2 to see if I can do it

@iandundas
Copy link
Contributor Author

iandundas commented Jan 25, 2017

Here's my rather naïve implementation:

extension ObservableArray where Item: Equatable {
  
  public func sorted(by sorter: @escaping (Item, Item) -> Bool) -> Signal<ObservableArrayEvent<Item>, NoError>{
    return Signal { (observer: AtomicObserver<ObservableArrayEvent<Item>, NoError>) -> Disposable in
      
      let disposable = DisposeBag()
      var differ: MutableObservableArray<Item>! // Stores a sorted version of the datasource, generates diff events
      
      self.observeNext(with: { (latestUpdate: ObservableArrayEvent<Item>) in
        let sorted = latestUpdate.dataSource.array.sorted(by: sorter)
        
        if differ == nil {
          differ = MutableObservableArray<Item>(latestUpdate.dataSource.array)
          differ
            // we don't want to pass on the first .reset event containing the unsorted array
            .skip(first: 1)
            // pass events from the differ to the signal observer:
            .observeNext(with: observer.next).dispose(in: disposable)
          
          // create our own .reset event instead, with the sorted array:
          observer.next(ObservableArrayEvent(change: .reset, source: sorted))
        }
        else {
          guard let differ = differ else {return}
          
          // Replace contents of differ to generate diff event for observer:
          differ.replace(with: sorted, performDiff: true)
        }
      }).dispose(in: disposable)
      
      return disposable
    }
  }
}

Usage:

let collection = MutableObservableArray<Int>([])

let sorted: Signal<ObservableArrayEvent<Int>, NoError> = collection.sorted { (a, b) -> Bool in
    return a < b
}

sorted.observeNext { (event: ObservableArrayEvent<Int>) in
    print("Received: \(event.change), array: \(event.dataSource.array)")

    switch event.change {
        case .reset: fallthrough
        case .endBatchEditing: print("")
        default: break
    }
}

collection.append(10)
collection.append(3)
collection.append(2)
collection.append(1)
collection.moveItem(from: 0, to: 2)
collection.remove(at: 1)
collection.remove(at: 2)

Outputs:

Received: reset, array: []

Received: beginBatchEditing, array: []
Received: inserts([0]), array: [10]
Received: endBatchEditing, array: [10]

Received: beginBatchEditing, array: [10]
Received: inserts([0]), array: [3, 10]
Received: endBatchEditing, array: [3, 10]

Received: beginBatchEditing, array: [3, 10]
Received: inserts([0]), array: [2, 3, 10]
Received: endBatchEditing, array: [2, 3, 10]

Received: beginBatchEditing, array: [2, 3, 10]
Received: inserts([0]), array: [1, 2, 3, 10]
Received: endBatchEditing, array: [1, 2, 3, 10]

Received: beginBatchEditing, array: [1, 2, 3, 10]
Received: endBatchEditing, array: [1, 2, 3, 10]

Received: beginBatchEditing, array: [1, 2, 3, 10]
Received: deletes([1]), array: [1, 3, 10]
Received: endBatchEditing, array: [1, 3, 10]

Received: beginBatchEditing, array: [1, 3, 10]
Received: deletes([0]), array: [3, 10]
Received: endBatchEditing, array: [3, 10]

Whereas the output if observing the collection (unsorted) directly is:

Received: reset, array: []

Received: inserts([0]), array: [10]
Received: inserts([1]), array: [10, 3]
Received: inserts([2]), array: [10, 3, 2]
Received: inserts([3]), array: [10, 3, 2, 1]
Received: move(0, 2), array: [3, 2, 10, 1]
Received: deletes([1]), array: [3, 10, 1]
Received: deletes([2]), array: [3, 10]

So the difference is an overuse of the begin/endBatchEditing (coming from the internal use of replace(with: sorted, performDiff: true)), but I can't see that doing much harm. In terms of performance it's probably not ideal I guess.

Would you be interested in me submitting a pull request for this with tests, or is there another implementation path that could be better?

@iandundas
Copy link
Contributor Author

updated the above to make examples better

@srdanrasic
Copy link
Contributor

I'll need few days to think about this :) I agree though, it would be nice to see sorting again.

@iandundas
Copy link
Contributor Author

Cool 👍🏻

@iandundas
Copy link
Contributor Author

Hey - did you have further ideas on this one?

@srdanrasic
Copy link
Contributor

Hi @iandundas, I didn't get much time to work on this recently. I want to do it but not sure when I'll get time. We would need a bit more performant implementation.

@slavikus
Copy link
Contributor

slavikus commented Mar 5, 2017

I'll watch this ticket as I'd love to see sortable arrays as well :)

@iandundas
Copy link
Contributor Author

iandundas commented Mar 6, 2017

hey @srdanrasic no worries :) fingers crossed

@ghost
Copy link

ghost commented Apr 16, 2017

Are there any updates on this? By the way. awesome library!

@slavikus
Copy link
Contributor

@srdanrasic @tonyarnold your magic is needed! :))

@srdanrasic
Copy link
Contributor

Sorting is back!

@iandundas
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants