Implement HashMap similar to Python's dict #19
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
src/hashable.ts
Outdated
| export abstract class Hashable { | ||
| abstract strictEquals(other: Hashable): boolean; | ||
|
|
||
| abstract valueOf(): number; |
There was a problem hiding this comment.
It’s not obvious that this returns specifically the hash value, I’d name it like getHash or getHashValue[Of]
There was a problem hiding this comment.
.valueOf() is something that every JS object kind of should have and is close enough to a hash in the identification sense.
src/hashable.ts
Outdated
| * Python style set implementation using hashable values. | ||
| */ | ||
| export class HashSet<T extends Hashable | Primitive> implements Set<T> { | ||
| private hashmap: HashMap<T, undefined>; |
There was a problem hiding this comment.
Is undefined really a good singleton type for non-values associated with present keys? I see you using undefined in HashMap.get (I presume in accordance with JS doing this for objects and stuff) so it’s better to use another singleton like null or maybe even true if it can be detached from the boolean type, than to ensure no undefined leaked somewhere.
| @@ -382,3 +382,31 @@ describe('Constant structure checker with a margin of equivalence', () => { | |||
| expect(hasMarginConstantStructure([1199, 1200], 2)).toBe(false); | |||
| }); | |||
| }); | |||
|
|
|||
There was a problem hiding this comment.
A good idea would be adding tests to cover every method of HashMap (but probably not necessary HashSet) because there are usually two main paths for Hashable and primitive types, so it’s good to at least be confident they don’t diverge in behavior.
|
Wait I think there was one more comment but well probably I’m seeing things. |
src/fraction.ts
Outdated
| * new Fraction("19.5").strictEquals(new Fraction("98/5")) // false | ||
| * ``` | ||
| **/ | ||
| strictEquals(other: Fraction) { |
There was a problem hiding this comment.
This one fails when using heterogenous HashMaps with other non-primitive keys.
| /** | ||
| * A hashable type expected to remain immutable so that it can be used as a dictionary / hash map key. | ||
| */ | ||
| export abstract class Hashable { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
A benchmark should be added to determine the impact of |
Implement an abstract base class for values hashable by their numeric value and distinguishable by strict equality. Implement HashMap as an associative data structure for hashable values. Implement HashSet as a collection of hashable values. Replace FractionSet implementation with HashSet<Fraction>.
|
This a bit more work than expected as the whole library needs more type precision when it comes to immutability. Converting PR to draft for later when |
Implement an abstract base class for values hashable by their numeric value and distinguishable by strict equality. Implement HashMap as an associative data structure for hashable values. Implement HashSet as a collection of hashable values. Replace FractionSet implementation with HashSet.