Skip to content

Add manual implementation of the Hashable protocol#4

Open
magauran wants to merge 1 commit intoJohnSundell:masterfrom
magauran:master
Open

Add manual implementation of the Hashable protocol#4
magauran wants to merge 1 commit intoJohnSundell:masterfrom
magauran:master

Conversation

@magauran
Copy link
Copy Markdown

@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
Copy Markdown
Author

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

@s4cha
Copy link
Copy Markdown

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

Development

Successfully merging this pull request may close these issues.

2 participants