Skip to content

Indices are (wrongly?) assumed to be integers and zero-based #37

Closed
@ole

Description

@ole

I have a comment regarding terminology.

The indices the library computes for the Edit steps are currently always integers and always zero-based (i.e. the first index is 0). This works great for the intended main use case (table and collection view updates) because the index paths of a table or collection view are also zero-based and composed of integers.

However, it doesn't reflect the reality of Swift's collection types. For example, ArraySlice uses integer indices, but they aren't zero-based. If you compute the changeset for two ArraySlice instances, the result is arguably wrong or at least confusing:

let source = [2,3,4,5].dropFirst() // ArraySlice<Int>
let target = [3,4,5,6].dropFirst() // ArraySlice<Int>
let edits = Changeset.edits(from: source, to: target)
print(edits)
// Prints "[delete 3 at index 0, insert 6 at index 2]"

If you tried to apply this changeset literally to the source collection (i.e. delete the element at index 0 from source), youʼd get a crash because source doesnʼt have an "index" 0.

In a way, a similar issue is when you compute changesets for strings (as most of the unit tests do) because string indices are not integers, so saying something like "insert "a" at index 5" doesn't strictly make sense in Swift.

I find this a little confusing and misleading. Changeset appears to be generic for any collection, and yet it doesn't follow Swift's conventions where the term "index" has a very specific meaning.

When I first noticed this "problem" I wanted to suggest replacing the integer indices in Edit and EditOperation with the collection's index type (i.e. something like T.Index). On second thought, this doesn't make sense because a changeset would have to compute "virtual" indices for a collection that doesn't really exist, and since collections are free to invalidate any existing index upon mutation, I don't think you could compute the indices in a generic way. Moreover, using real collection indices would make supporting table/collection view updates harder, not easier.

After writing all this, I realize that the way this works is actually the best solution for the given problem. Having written this, I'm going to submit it too, even though I don't have a specific suggestion. Feel free to close this issue, and maybe it will help others understand this better when they find it in search.

My only suggestion would be to make it clear that Changeset and Edit aren't concerned with indices, but with offsets. I think if the debugDescription read "delete x at offset 0" etc. I would have been less confused. This is exemplified by the use of enumerated() to compute the column value in edits(from:to:), since enumerated() computes offsets from 0 and not indices (see http://khanlou.com/2017/03/you-probably-don't-want-enumerated/ for more info on enumerated() and how it's often misinterpreted).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions