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

Support moves #70

Open
eliasbagley opened this issue Aug 10, 2017 · 3 comments
Open

Support moves #70

eliasbagley opened this issue Aug 10, 2017 · 3 comments

Comments

@eliasbagley
Copy link

First off, great library! I've struggled with this issue for years and have always fallen back to an NSFetchedResultsController or just calling reloadData.

I watched your meetup video about Dwifft 0.6 online, and saw you mentioned having support for move operations for 1.0, which would be rad! I just wanted to make you aware of the Android implementation of this functionality (if you aren't already) - maybe you can take some inspiration from it, since it supports moves, inserts, and deletes. I think it's a different algorithm than the one you're using, but might be useful. The docs say it uses two passes of the Myers Difference Algorithm (which I believe is the same LCS algorithm you're using?) in order to detect the moves, since it can't be done in a single pass.

https://developer.android.com/reference/android/support/v7/util/DiffUtil.html

Keep up the good work!

@jack-stripe
Copy link
Collaborator

@eliasbagley - thank you! wow, i had no idea this was in android; that is super helpful. I've looked over the DiffUtil source a bit, but it'll take a more thorough read to see how easy it'd be to steal port their move reconciliation stuff over to Dwifft. Thanks again for the link!

@marcprux
Copy link

marcprux commented Dec 2, 2018

This is pretty easy to do as a post-processing step by collapsing .insert and .delete with the same item into a single .move. This is the approach that DeepDiff takes (see https://github.com/onmyway133/DeepDiff/blob/master/Sources/Shared/MoveReducer.swift).

I've done a quick implementation below, and it can be used by changing the sectionedValues updater to be:

  let diff = Dwifft.synthesizeMoves(Dwifft.diff(lhs: oldSectionedValues, rhs: newSectionedValues))

I've implemented it in my own NSCollectionView, and it seems to have tolerable performance (plus it looks great!). I could probably put together a PR if there is sufficient interest.

public extension Dwifft {

    /// Takes a series of diff steps and collapses matching inserts and deletes into a single move operation.
    ///
    /// Note that this requires adding a case move(Int, Int, Value, Int, Int) to SectionedDiffStep
    /// and processing moves appropriately in the various AbstractDiffCalculator implementations, e.g.:
    ///   case let .move(fromSection, fromItem, _, toSection, toItem):
    ///       collectionView.moveItem(at: IndexPath(item: fromItem, section: fromSection), to: IndexPath(item: toItem, section: toSection))
    static func synthesizeMoves<Section, Value: Equatable>(_ source: [SectionedDiffStep<Section, Value>]) -> [SectionedDiffStep<Section, Value>] {
        typealias Mutation = (offset: Int, element: (section: Int, index: Int, value: Value))

        func insertMutation(offset: Int, forStep: SectionedDiffStep<Section, Value>) -> Mutation? {
            if case let .insert(section, index, value) = forStep { return (offset, (section, index, value)) } else { return nil }
        }

        func deleteMutation(offset: Int, forStep: SectionedDiffStep<Section, Value>) -> Mutation? {
            if case let .delete(section, index, value) = forStep { return (offset, (section, index, value)) } else { return nil }
        }

        // this could be a set, but we might have the same value more than once in the model
        let deleteValues = source.enumerated().compactMap(deleteMutation).map({ $0.element.value })

        // no deletes means no moves
        if deleteValues.isEmpty { return source }

        var changes = source

        for candidate in deleteValues {
            guard let deleteItem = changes.lazy.enumerated().compactMap(deleteMutation).first(where: { $0.element.value == candidate }) else { continue }
            guard let insertItem = changes.lazy.enumerated().compactMap(insertMutation).first(where: { $0.element.value == candidate }) else { continue }

            let (lower, upper) = (min(deleteItem.offset, insertItem.offset), max(deleteItem.offset, insertItem.offset))

            changes.remove(at: upper)
            changes.remove(at: lower)
            changes.insert(.move(deleteItem.element.section, deleteItem.element.index, candidate, insertItem.element.section, insertItem.element.index), at: lower)
        }

        return changes
    }
}

@Alex293
Copy link

Alex293 commented Dec 3, 2018

+1 @marcprux

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

No branches or pull requests

4 participants