Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Unexpected behavior when using the default implementations of Differentiable for Hashable #114

Open
yunhao opened this issue Sep 11, 2020 · 1 comment · May be fixed by #115
Open

Unexpected behavior when using the default implementations of Differentiable for Hashable #114

yunhao opened this issue Sep 11, 2020 · 1 comment · May be fixed by #115

Comments

@yunhao
Copy link

yunhao commented Sep 11, 2020

DifferenceKit provides a default implementation of Differentiable for Hashable. However, when using the default implementation, DifferenceKit always trigers a delete -> insert change instead of a update change.

Unexpected

I have a simple model that uses the default differenceIdentifier implementation.

struct ElementModel {
    // The identifier.
    var id: Int
    
    // The contents.
    var title: String
    var value: String
}

// Use the default `differenceIdentifier` implementation.
extension ElementModel: Hashable, Differentiable {
    func hash(into hasher: inout Hasher) {
        hasher.combine(id)
    }
}

Create a changeset between two items that have the same id but different value.

let list1 = [ElementModel(id: 1, title: "color", value: "red")]
let list2 = [ElementModel(id: 1, title: "color", value: "blue")]

let changeset = StagedChangeset(source: list1, target: list2)
changeset.forEach { print($0) }

Unfortunately, the output message shows that there is a delete -> insert update.

Changeset(
    data: [],
    elementDeleted: [
        [element: 0, section: 0]
    ]
)
Changeset(
    data: [
        ElementModel(id: 1, title: "color", value: "blue")
    ],
    elementInserted: [
        [element: 0, section: 0]
    ]
)

Expected

If I explicitly implement the differenceIdentifier property by return hashValue, everything will be fine.

extension ElementModel: Hashable, Differentiable {
    func hash(into hasher: inout Hasher) {
        hasher.combine(id)
    }
    
    var differenceIdentifier: Int {
        // Return `hashValue` insteal of `Self`
        return hashValue
    }
}

The expected update change.

Changeset(
    data: [
        ElementModel(id: 1, title: "color", value: "blue")
    ],
    elementUpdated: [
        [element: 0, section: 0]
    ]
)

Question

I'm not sure if this is a mistake of my code of a bug of DifferenceKit.

DifferenceKit provides a default implementation of Differentiable for Hashable, but the implementation of differenceIdentifier just return the Hashable instance itself. I think maybe the hashValue is the right return value.

// Return self
var differenceIdentifier: Self {
    return self
}

// Return hashValue
var differenceIdentifier: Int {
    return hashValue
}
@yunhao
Copy link
Author

yunhao commented Sep 11, 2020

After clone this project and modify the implementation to return hashValue, the unit test fails at func testSameHashValue(). I'm a little confused about this test case.

func testSameHashValue() {
struct A: Hashable, Differentiable {
let actualValue: Int
init(_ actualValue: Int) {
self.actualValue = actualValue
}
func hash(into hasher: inout Hasher) {
hasher.combine(0)
}
func isContentEqual(to source: A) -> Bool {
return actualValue == source.actualValue
}
}
let section = 0
let source = [A(0), A(1)]
let target = [A(0), A(2)]
XCTAssertExactDifferences(
source: source,
target: target,
section: section,
expected: [
Changeset(
data: [A(0)],
elementDeleted: [ElementPath(element: 1, section: section)]
),
Changeset(
data: target,
elementInserted: [ElementPath(element: 1, section: section)]
)
]
)
}
}

I think it should be a update change instead of a delete -> insert change, because they have the same identifier (hasher.combine(0)) and different content.

It seems that DifferenceKit doesn't use the result of func hash(into hasher: inout Hasher). When using Hashable, we can use hasher.combine(identifier) to determine our identifier, but DifferenceKit will ignore it.

@yunhao yunhao linked a pull request Sep 11, 2020 that will close this issue
4 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant