-
Notifications
You must be signed in to change notification settings - Fork 87
Changes from all commits
b78de08
8f961ff
88aa113
b6ce4b5
72a6789
4ef876d
0bbee8d
c722cdc
6ac3b5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ import 'dart:collection'; | |
/// more efficient than a [LinkedHashMap] with a custom equality operator | ||
/// because it only canonicalizes each key once, rather than doing so for each | ||
/// comparison. | ||
class CanonicalizedMap<C, K, V> implements Map<K, V> { | ||
final class CanonicalizedMap<C, K, V> implements Map<K, V> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class has subclasses in the ecosystem. I think this looks like a legitimately useful pattern. Perhaps this class could be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess even marking it as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Why is making it That subclass doesn't really need to be a class. It's an implementation strategy for the It could just be a function: Map<String, V> caseInsensitiveMap<V>({Map<String, V>? from}) =>
from == null
? CanonicalizedMap<String, String, V>(_toLowerCase)
: CanonicalizedMap<String, String, V>.from(from, _toLowerCase); That would be breaking, since it's not how it was originally written, and we can't really replace the unnamed constructor with something returning a different type.) But it can use a wrapper and delegation and preserve the unnecessary class: final class CaseInsensitiveMap<V> extends DelegatingMap<String, V> {
CaseInsensitiveMap() : super(CanonicalizedMap<String, String, V>(_toLowerCase));
CaseInsensitiveMap.from(Map<String, V> other)
: super(CanonicalizedMap<String, String, V>.from(from, _toLowerCase));
static String _toLowerCase(String string) => string.toLowerCase();
} In either case, the result doesn't need to be-a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It could be a quirk of google3 - when I made it
Yeah, it's not a good design and we should fix it. We just need to decide what types of misuses that exist in google3 or that we can find in pub that we want to avoid breaking, and which we are OK with breaking. |
||
final C Function(K) _canonicalize; | ||
|
||
final bool Function(K)? _isValidKeyFn; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ import 'comparators.dart'; | |
const int _hashMask = 0x7fffffff; | ||
|
||
/// A generic equality relation on objects. | ||
abstract class Equality<E> { | ||
abstract interface class Equality<E> { | ||
const factory Equality() = DefaultEquality<E>; | ||
|
||
/// Compare two elements for being equal. | ||
|
@@ -46,7 +46,7 @@ abstract class Equality<E> { | |
/// | ||
/// It's also possible to pass an additional equality instance that should be | ||
/// used to compare the value itself. | ||
class EqualityBy<E, F> implements Equality<E> { | ||
final class EqualityBy<E, F> implements Equality<E> { | ||
final F Function(E) _comparisonKey; | ||
|
||
final Equality<F> _inner; | ||
|
@@ -81,7 +81,7 @@ class EqualityBy<E, F> implements Equality<E> { | |
/// Note that [equals] and [hash] take `Object`s rather than `E`s. This allows | ||
/// `E` to be inferred as `Null` in const contexts where `E` wouldn't be a | ||
/// compile-time constant, while still allowing the class to be used at runtime. | ||
class DefaultEquality<E> implements Equality<E> { | ||
final class DefaultEquality<E> implements Equality<E> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intended an implementation class for the I can see how this can also be used as a skeleton implementation, where you override only the parts that differ from default equality. In which case making it open makes sense. |
||
const DefaultEquality(); | ||
@override | ||
bool equals(Object? e1, Object? e2) => e1 == e2; | ||
|
@@ -92,7 +92,7 @@ class DefaultEquality<E> implements Equality<E> { | |
} | ||
|
||
/// Equality of objects that compares only the identity of the objects. | ||
class IdentityEquality<E> implements Equality<E> { | ||
final class IdentityEquality<E> implements Equality<E> { | ||
const IdentityEquality(); | ||
@override | ||
bool equals(E e1, E e2) => identical(e1, e2); | ||
|
@@ -109,7 +109,7 @@ class IdentityEquality<E> implements Equality<E> { | |
/// The [equals] and [hash] methods accepts `null` values, | ||
/// even if the [isValidKey] returns `false` for `null`. | ||
/// The [hash] of `null` is `null.hashCode`. | ||
class IterableEquality<E> implements Equality<Iterable<E>> { | ||
final class IterableEquality<E> implements Equality<Iterable<E>> { | ||
final Equality<E?> _elementEquality; | ||
const IterableEquality( | ||
[Equality<E> elementEquality = const DefaultEquality<Never>()]) | ||
|
@@ -161,7 +161,7 @@ class IterableEquality<E> implements Equality<Iterable<E>> { | |
/// The [equals] and [hash] methods accepts `null` values, | ||
/// even if the [isValidKey] returns `false` for `null`. | ||
/// The [hash] of `null` is `null.hashCode`. | ||
class ListEquality<E> implements Equality<List<E>> { | ||
final class ListEquality<E> implements Equality<List<E>> { | ||
final Equality<E> _elementEquality; | ||
const ListEquality( | ||
[Equality<E> elementEquality = const DefaultEquality<Never>()]) | ||
|
@@ -202,7 +202,7 @@ class ListEquality<E> implements Equality<List<E>> { | |
bool isValidKey(Object? o) => o is List<E>; | ||
} | ||
|
||
abstract class _UnorderedEquality<E, T extends Iterable<E>> | ||
abstract final class _UnorderedEquality<E, T extends Iterable<E>> | ||
implements Equality<T> { | ||
final Equality<E> _elementEquality; | ||
|
||
|
@@ -251,7 +251,8 @@ abstract class _UnorderedEquality<E, T extends Iterable<E>> | |
/// Two iterables are considered equal if they have the same number of elements, | ||
/// and the elements of one set can be paired with the elements | ||
/// of the other iterable, so that each pair are equal. | ||
class UnorderedIterableEquality<E> extends _UnorderedEquality<E, Iterable<E>> { | ||
final class UnorderedIterableEquality<E> | ||
extends _UnorderedEquality<E, Iterable<E>> { | ||
const UnorderedIterableEquality( | ||
[super.elementEquality = const DefaultEquality<Never>()]); | ||
|
||
|
@@ -271,7 +272,7 @@ class UnorderedIterableEquality<E> extends _UnorderedEquality<E, Iterable<E>> { | |
/// The [equals] and [hash] methods accepts `null` values, | ||
/// even if the [isValidKey] returns `false` for `null`. | ||
/// The [hash] of `null` is `null.hashCode`. | ||
class SetEquality<E> extends _UnorderedEquality<E, Set<E>> { | ||
final class SetEquality<E> extends _UnorderedEquality<E, Set<E>> { | ||
const SetEquality([super.elementEquality = const DefaultEquality<Never>()]); | ||
|
||
@override | ||
|
@@ -282,7 +283,7 @@ class SetEquality<E> extends _UnorderedEquality<E, Set<E>> { | |
/// | ||
/// The class represents a map entry as a single object, | ||
/// using a combined hashCode and equality of the key and value. | ||
class _MapEntry { | ||
final class _MapEntry { | ||
final MapEquality equality; | ||
final Object? key; | ||
final Object? value; | ||
|
@@ -309,7 +310,7 @@ class _MapEntry { | |
/// The [equals] and [hash] methods accepts `null` values, | ||
/// even if the [isValidKey] returns `false` for `null`. | ||
/// The [hash] of `null` is `null.hashCode`. | ||
class MapEquality<K, V> implements Equality<Map<K, V>> { | ||
final class MapEquality<K, V> implements Equality<Map<K, V>> { | ||
final Equality<K> _keyEquality; | ||
final Equality<V> _valueEquality; | ||
const MapEquality( | ||
|
@@ -372,7 +373,7 @@ class MapEquality<K, V> implements Equality<Map<K, V>> { | |
/// for `equals(e1, e2)` and `equals(e2, e1)`. This can happen if one equality | ||
/// considers only `e1` a valid key, and not `e2`, but an equality which is | ||
/// checked later, allows both. | ||
class MultiEquality<E> implements Equality<E> { | ||
final class MultiEquality<E> implements Equality<E> { | ||
final Iterable<Equality<E>> _equalities; | ||
|
||
const MultiEquality(Iterable<Equality<E>> equalities) | ||
|
@@ -418,7 +419,7 @@ class MultiEquality<E> implements Equality<E> { | |
/// | ||
/// A list is only equal to another list, likewise for sets and maps. All other | ||
/// iterables are compared as iterables only. | ||
class DeepCollectionEquality implements Equality { | ||
final class DeepCollectionEquality implements Equality { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mockito extends this class. We could rewrite that to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see how the recursive behavior of creating equalities with A subclass can override So, it's actually a reasonable use-case to subclass and extend. Let's open it. |
||
final Equality _base; | ||
final bool _unordered; | ||
const DeepCollectionEquality([Equality base = const DefaultEquality<Never>()]) | ||
|
@@ -476,7 +477,7 @@ class DeepCollectionEquality implements Equality { | |
/// String equality that's insensitive to differences in ASCII case. | ||
/// | ||
/// Non-ASCII characters are compared as-is, with no conversion. | ||
class CaseInsensitiveEquality implements Equality<String> { | ||
final class CaseInsensitiveEquality implements Equality<String> { | ||
const CaseInsensitiveEquality(); | ||
|
||
@override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file | ||
// for details. All rights reserved. Use of this source code is governed by a | ||
// BSD-style license that can be found in the LICENSE file. | ||
|
||
extension MapExtensions<K, V> on Map<K, V> { | ||
/// Like [Map.entries], but returns each entry as a record. | ||
Iterable<(K, V)> get pairs => entries.map((e) => (e.key, e.value)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting...but why? What's the upside here? What do users get? We're already going to allocate (however short-lived) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main benefit is being able to loop over pairs with variables named something other than "key" and "value". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we're still hoping to make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I doubt it will be over |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ import 'utils.dart'; | |
/// If elements override [Object.==], the `comparison` function must | ||
/// always give equal objects the same priority, | ||
/// otherwise [contains] or [remove] might not work correctly. | ||
abstract class PriorityQueue<E> { | ||
abstract interface class PriorityQueue<E> { | ||
/// Creates an empty [PriorityQueue]. | ||
/// | ||
/// The created [PriorityQueue] is a plain [HeapPriorityQueue]. | ||
|
@@ -168,7 +168,7 @@ abstract class PriorityQueue<E> { | |
/// and is linear, O(n). | ||
/// * The [toSet] operation effectively adds each element to the new set, taking | ||
/// an expected O(n*log(n)) time. | ||
class HeapPriorityQueue<E> implements PriorityQueue<E> { | ||
final class HeapPriorityQueue<E> implements PriorityQueue<E> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is an internal class which extends this. /// A [PriorityQueue] that works like a normal queue for elements that compare
/// as equal, that is, returns them in the order they were added.
///
/// Takes an optional [Comparator] parameter that should be used instead of the
/// default [Comparable.compare].
class _StablePriorityQueue<T extends Comparable<Object>>
extends HeapPriorityQueue<T> { It seems to record the values that are added in a second collection and uses it to set the order for "equal" elements. At first glance I don't see a clean way to implement it without There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not a very good example of subclassing. For example that implementation doesn't work correctly if the same item is added more than once, and there are other elements which compare equal (comparison result zero), but are not And it's lucky that the implementation of Wrapping seems like it would be easier and safer than subclassing in this situation. ("Favor composition over inheritance"!) import "package:collection/collection.dart" show HeapPriorityQueue;
class _StablePriorityQueue<T> {
final HeapPriorityQueue<(T, int)> _queue;
int _counter = 0;
_StablePriorityQueue(Comparator<T> comparison)
: _queue = HeapPriorityQueue(_orderedComparator<T>(comparison));
static int Function((T, int), (T, int)) _orderedComparator<T>(
int Function(T, T) comparison) =>
((T, int) a, (T, int) b) {
var result = comparison(a.$1, b.$1);
if (result != 0) return result;
return a.$2.compareTo(b.$2); // or just: return a.$2 - b.$2;
};
void add(T element) {
_queue.add((element, _counter++));
}
bool get isNotEmpty => _queue.isNotEmpty;
T removeFirst() => _queue.removeFirst().$1;
void clear() {
_queue.clear();
_counter = 0; // For good measure, not really needed.
}
} It's a private class, so by not extending a larger interface, it only needs to care about the features that are actually needed by the code in the same library. If it doesn't use It even works if you add the same element more than once, because the comparison is stable and based only on the values in the queue, not extra data on the side. After talking it up so much, I' guess I should just go and make the change 😁 |
||
/// Initial capacity of a queue when created, or when added to after a | ||
/// [clear]. | ||
/// | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,7 @@ class NonGrowableListView<E> extends DelegatingList<E> | |
|
||
/// Mixin class that implements a throwing version of all list operations that | ||
/// change the List's length. | ||
abstract class NonGrowableListMixin<E> implements List<E> { | ||
abstract mixin class NonGrowableListMixin<E> implements List<E> { | ||
natebosch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
static Never _throw() { | ||
throw UnsupportedError('Cannot change the length of a fixed-length list'); | ||
} | ||
|
@@ -116,7 +116,7 @@ class UnmodifiableSetView<E> extends DelegatingSet<E> | |
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not to self: The |
||
/// Mixin class that implements a throwing version of all set operations that | ||
/// change the Set. | ||
abstract /*mixin*/ class UnmodifiableSetMixin<E> implements Set<E> { | ||
abstract mixin class UnmodifiableSetMixin<E> implements Set<E> { | ||
static Never _throw() { | ||
throw UnsupportedError('Cannot modify an unmodifiable Set'); | ||
} | ||
|
@@ -164,7 +164,7 @@ abstract /*mixin*/ class UnmodifiableSetMixin<E> implements Set<E> { | |
|
||
/// Mixin class that implements a throwing version of all map operations that | ||
/// change the Map. | ||
abstract /*mixin*/ class UnmodifiableMapMixin<K, V> implements Map<K, V> { | ||
abstract mixin class UnmodifiableMapMixin<K, V> implements Map<K, V> { | ||
static Never _throw() { | ||
throw UnsupportedError('Cannot modify an unmodifiable Map'); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version was never published, so we can probably just drop the changelog entry.