Underspecified Set<T> and Map<K,V> semantics #472
Comments
|
This comment was originally written by alexey.v.varl...@gmail.com There is an issue #167, which may be relevant to this one, yet these issues are not close duplicates. This issue highlights particular flaw in Set&Map declaration so is a bug rather than enhancement. |
|
The interfaces Map and Set are currently underspecified. The specification should be: interface Set<E> extends Collection<E> factory HashSetImplementation<E extends Hashable> { interface Map<K, V> factory HashMapImplementation<K extends Hashable, V> { However, none of our implementations allow this syntax yet. So you'll have to fill it in yourself when reading the API :-) Removed Type-Defect label. |
|
This comment was originally written by alexey.v.var...@gmail.com Hmm, the chosen approach to add extra type bounds to the factory clause seems to be mere "technical" (or, syntactical) solution. Conceptually, it is still a bit weird to define a factory for an interface with incomplete contract - and apparently the choice of equivalence function is a significant part of Set meaning. |
|
STRONGLY agreed w/ Comment #3 from Alex. If Set<E> and Map<K, V> doen't explicitly require Hashable keys, then their default implementations should not require Hashable keys. Period. Better to eliminate the default implementations entirely and push folks towards HashSet and HashMap respectively. The current implementation feels VERY broken. |
|
Given that Hashable has gone away (everything is hashable) I think this issue can be closed. |
|
Not a problem anymore since everything is now hashable. That said: we are going to add means to provide equivalence functions to the map and set. Added AssumedStale label. |
|
Excellent. |
DartBot
added
Type-Enhancement
area-core-library
kevmoo
added
type-enhancement
Allow analyzer master failures in Travis
Changes: ``` > git log --format="%C(auto) %h %s" 93d0eee..49eefd2 https://dart.googlesource.com/markdown.git/+/49eefd2 Refactor AutolinkExtensionSyntax (#471) https://dart.googlesource.com/markdown.git/+/07e2683 Optimise TableSyntax (#472) https://dart.googlesource.com/markdown.git/+/9b61871 Make helper class private that should not have been exposed (#476) https://dart.googlesource.com/markdown.git/+/299964e Return list for link nodes creation (#452) https://dart.googlesource.com/markdown.git/+/aee6a40 validate code coverage on CI (#474) https://dart.googlesource.com/markdown.git/+/88f3f8a Fix html entity and numeric character references (#467) ``` Diff: https://dart.googlesource.com/markdown.git/+/93d0eee771f6355be6737c2a865f613f6b105bf1~..49eefd211e7840bac7e11257cd966435ae3cb07f/ Change-Id: I2a88d7c386f567738226701be4edcd7c4818744f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/266760 Auto-Submit: Devon Carew <devoncarew@google.com> Commit-Queue: Oleh Prypin <oprypin@google.com> Reviewed-by: Oleh Prypin <oprypin@google.com>
This issue was originally filed by alexey.v.var...@gmail.com
The corelib interface Set<T> does not define any particular way to distinguish objects, for checking uniqueness. Given there is also more specialized interface HashSet<E extends Hashable>, the Set is naturally assumed as a generic container for objects of any type.
However current Set declaration refers to factory HashSetImplementation<E>. As class Object does not implement Hashable, nor the interface Set mentions Hashable as a type bound, this provokes confusion and induces unexpected runtime errors.
The same issue applies to Map<K,V> vs HashMap<K extends Hashable, V>.
Suggested solutions:
Add equivalence function as a parameter to Set (Map), and provide a proper factory class for it.
Alternative (half-)solution:
Drop the "factory" clause from the generic Set (Map) and leave it as a pure abstraction, for future extensions. The HashSet (HashMap) remains a viable implementation with clear semantics.
Another alternative solution:
Make all objects hashable. This would address the Set/HashSet controversy as well.
The text was updated successfully, but these errors were encountered: