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

Add manual implementation of the Hashable protocol #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

magauran
Copy link

@magauran magauran commented Mar 9, 2020

Models with different types can have ids with the same rawValue. In the current implementation, such ids have the same hashValue.

It can cause problems and inefficiencies wherever the hashValue is used (Dictionary, Set and others).

For example:

protocol Model {}

struct User: Identifiable, Model {
    let id: ID
}

struct Offer: Identifiable, Model {
    let id: ID
}

let user = User(id: "0")
let offer = Offer(id: "0")

var dictionary = [AnyHashable: Model]()
dictionary[user.id] = user
dictionary[offer.id] = offer

print(user.id.hashValue) // -8325541509441078555
print(offer.id.hashValue) // -8325541509441078555
print(AnyHashable(user.id).hashValue) // -8325541509441078555
print(AnyHashable(offer.id).hashValue) // -8325541509441078555

This example causes collision in the dictionary. The collision will be successfully resolved because Equatable implementation in AnyHashable checks that compared items have the same type.

print(AnyHashable(user.id) == AnyHashable(offer.id)) // false

But it is bad for computable complexity.

Computational Complexity
The access time for a value in the dictionary is guaranteed to be at worst O(N) for any implementation, current and future, but will often be O(1) (constant time). Insertion or deletion operations will typically be constant time as well, but are O(N*N) in the worst case in some implementations. Access of values through a key is faster than accessing values directly (if there are any such operations). Dictionaries will tend to use significantly more memory than a array with the same number of values.

I propose considering the type of Identifier.Value when calculating the hash.

@magauran
Copy link
Author

Hi, @JohnSundell! What do you think about it?

@s4cha
Copy link

s4cha commented Jun 10, 2020

From what I can see, this would make total sense to integrate the Type into the hash :)

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