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

Align Differentiable with Identifiable protocol #80

Open
2 tasks done
Marcocanc opened this issue Oct 19, 2019 · 3 comments
Open
2 tasks done

Align Differentiable with Identifiable protocol #80

Marcocanc opened this issue Oct 19, 2019 · 3 comments

Comments

@Marcocanc
Copy link
Contributor

Marcocanc commented Oct 19, 2019

Checklist

Description

Since the identity component of Differentiable is semantically identical to the new Identifiable protocol in Swift 5.1, I think it would be beneficial to align the two APIs.

Motivation and Context

This allows us to easily conform to Identifiable as well as Differentiable through a single implementation.

Proposed Solution

  • Separate Equality (ContentEquatable) and Identification (ContentIdentifiable) component through the use of protocol composition for Differentiable
  • Align the API of ContentIdentifiable to that of Identifiable

See #81 for implementation details

@ra1028
Copy link
Owner

ra1028 commented Oct 20, 2019

Thank you for your proposal @Marcocanc

As you described, Identifiable and Differentiable have semantically the same properties.
However, Identifiable has OS restrictions, and I think there are few benefits to use it with breaking API.
How about implementing such a simple extension?

public extension Differentiable where Self: Identifiable {
    @inlinable
    var differenceIdentifier: ID {
        id
    }
}

@Marcocanc
Copy link
Contributor Author

Thanks for the quick feedback. I understand that aligning the API might not be worth breaking existing code bases.

I have created a separate PR (#83) which introduces ContentIdentifiable and removes its dependency from ContentEquatable. Differentiable is now a composed protocol.
This will allow to work with the protocols on a more granular level and avoid any breaking APIs.

@ra1028
Copy link
Owner

ra1028 commented Oct 22, 2019

Thanks @Marcocanc

The changes by #83 breaks the existing API, but agrees that it can be further clarified by separating identification and equality, so I'll allow this and include it in the next version 1.2.0.

Whether to change the naming differenceIdentifier to id will be discussed again when DifferenceKit grows to version 2.0.0.

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

2 participants