-
Notifications
You must be signed in to change notification settings - Fork 87
Fix issue 355 added get frequency getter to iterable #357
Changes from all commits
ddcad5b
c2a7133
0c833d8
b444e37
1fe3d16
c5e18a2
8acd858
facd926
f4bbe43
9c4cf47
da963a5
2373566
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 |
---|---|---|
|
@@ -601,6 +601,41 @@ extension IterableExtension<T> on Iterable<T> { | |
yield slice; | ||
} | ||
} | ||
|
||
/// The count of occurrences of each element. | ||
/// | ||
/// The map contains an entry for each unique element of this iterable, | ||
/// as determined by `==`. | ||
/// The value for each key is the number of times that element | ||
/// appears in the iterable. | ||
/// | ||
/// If there are elements that are equal (`==`), | ||
/// but not identical (`identical`), | ||
/// it is unspecified which of the elements is used as the key in the map. | ||
/// For example, if there are multiple lists with the same content, | ||
/// the map will only keep one of them as a key. | ||
/// | ||
/// Example: | ||
/// ```dart | ||
/// ['a', 'b', 'c', 'b', 'c', 'c'].frequencies; | ||
/// Returns: {'a': 1, 'b': 2, 'c': 3}. | ||
/// ``` | ||
/// | ||
/// Note: This method uses `==` to compare elements. | ||
/// For collections # `List`, `Set`, or `Map`, deep equality is not checked. | ||
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. ... where not even shallow equality is checked, two lists are equal only if they are the same instance. If that is a use-case you want to support, maybe we should to add a parameter for the equality, to allow you to ask for Map<T, int> frequencies([Equality<T>? equality]) {
var frequenceyMap = equality == null ? <T, int>{} :
equality is IdentityEquality ? LinkedHashMap<T, int>.identity() :
LinkedHashMap<T, int>(equality.equals, equality.hash);
// ...
} or have a parallel Then you can do nested list equality 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. I think this is "shallow" as it relates to 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. ACK. In that case the wording is at least ambiguous. /// For elements that are collections, like `List`, `Set` or `Map`, the `==` comparison only
/// checks the identity of the collection, not its contents. |
||
/// If you need deep equality (e.g., nested lists), | ||
/// consider using a custom equality mechanism. | ||
|
||
Map<T, int> get frequencies { | ||
final frequencyMap = <T, int>{}; | ||
for (var item in this) { | ||
frequencyMap.update(item, _increment, ifAbsent: _one); | ||
} | ||
return frequencyMap; | ||
} | ||
|
||
static int _increment(int value) => value + 1; | ||
static int _one() => 1; | ||
} | ||
|
||
/// Extensions that apply to iterables with a nullable element type. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
// 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. | ||
|
||
import 'dart:collection'; | ||
import 'dart:math' show Random, pow; | ||
|
||
import 'package:collection/collection.dart'; | ||
|
@@ -1352,6 +1353,122 @@ void main() { | |
expect(l3.toList(), [4, 5]); | ||
}); | ||
}); | ||
group('get frequencies tests', () { | ||
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. Just The names of groups and tests are concatenated with a space between them, so they can rely on context. Things like "should be correct" is a given for tests, so it doesn't add value. Consider: group('frequencies', () {
test('of integers', () { ... });
test('of strings', () { ... });
test('of empty iterable', () { ... });
test('of empty single element iterable', () { ... });
group('(when equality is not identity)', () {
test('of records', () { ... });
/// ...
});
}); These names are only important when a test fails. Take "frequencies of single element iterable" with an error (if the result was empty) of
With that headline, it's fairly clear where and what the problem is. |
||
test('should return correct frequency map for List of integers', () { | ||
var list = [1, 2, 2, 3, 3, 3]; | ||
var frequencyMap = list.frequencies; | ||
expect(frequencyMap, {1: 1, 2: 2, 3: 3}); | ||
}); | ||
|
||
test('should return correct frequency map for List of strings', () { | ||
var list = ['a', 'b', 'b', 'c', 'c', 'c']; | ||
var frequencyMap = list.frequencies; | ||
expect(frequencyMap, {'a': 1, 'b': 2, 'c': 3}); | ||
}); | ||
|
||
test('should handle empty List', () { | ||
var list = []; | ||
var frequencyMap = list.frequencies; | ||
expect(frequencyMap, {}); | ||
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. Consider |
||
}); | ||
|
||
test('should handle single element List', () { | ||
var list = [42]; | ||
var frequencyMap = list.frequencies; | ||
expect(frequencyMap, {42: 1}); | ||
}); | ||
|
||
test('should return correct frequency map for Set of integers', () { | ||
// ignore: equal_elements_in_set | ||
var set = {1, 2, 2, 3, 3, 3}; | ||
var frequencyMap = set.frequencies; | ||
expect(frequencyMap, {1: 1, 2: 1, 3: 1}); | ||
}); | ||
|
||
test('should return correct frequency map for Set of strings', () { | ||
// ignore: equal_elements_in_set | ||
var set = {'a', 'b', 'b', 'c', 'c', 'c'}; | ||
var frequencyMap = set.frequencies; | ||
expect(frequencyMap, {'a': 1, 'b': 1, 'c': 1}); | ||
}); | ||
|
||
test('should handle empty Set', () { | ||
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 would drop the tests for sets and queues unless there is a reason to believe that the implementation treats them differently (which it doesn't). We're not testing iteration, that is assumed to work, so all that matters is the sequence of values we get by iterating, and a (If we start trying to detect lists or sets, to do more efficient iteration or maybe assuming that sets cannot have duplicates so we don't need to count - which isn't true because sets can have non-standard equality - then there is reason to test with, and without, those types specifically.) |
||
var set = <int>{}; | ||
var frequencyMap = set.frequencies; | ||
expect(frequencyMap, {}); | ||
}); | ||
|
||
test('should handle single element Set', () { | ||
var set = {42}; | ||
var frequencyMap = set.frequencies; | ||
expect(frequencyMap, {42: 1}); | ||
}); | ||
|
||
test('should return correct frequency map for Queue of integers', () { | ||
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 probably overkill to go throguh all kinds of iterables. For example having elements that are equal, but not identical, or having elements that are records. 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. added |
||
var queue = Queue<int>(); | ||
queue.addAll([1, 2, 2, 3, 3, 3]); | ||
var frequencyMap = queue.frequencies; | ||
expect(frequencyMap, {1: 1, 2: 2, 3: 3}); | ||
}); | ||
|
||
test('should return correct frequency map for Queue of strings', () { | ||
var queue = Queue<String>(); | ||
queue.addAll(['a', 'b', 'b', 'c', 'c', 'c']); | ||
var frequencyMap = queue.frequencies; | ||
expect(frequencyMap, {'a': 1, 'b': 2, 'c': 3}); | ||
}); | ||
|
||
test('should handle empty Queue', () { | ||
var queue = Queue<int>(); | ||
var frequencyMap = queue.frequencies; | ||
expect(frequencyMap, {}); | ||
}); | ||
|
||
test('should handle single element Queue', () { | ||
var queue = Queue<int>(); | ||
queue.add(42); | ||
var frequencyMap = queue.frequencies; | ||
expect(frequencyMap, {42: 1}); | ||
}); | ||
}); | ||
|
||
group('get frequencies tests extended', () { | ||
test('list of equal but not identical strings', () { | ||
var list = ['apple', String.fromCharCodes('apple'.codeUnits)]; | ||
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. String is not a good choice for this test because those will be identical when compiled for web. I'd drop this one, and just use the |
||
var frequencyMap = list.frequencies; | ||
expect(frequencyMap, {'apple': 2}); | ||
}); | ||
|
||
test('list of records', () { | ||
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. (So for naming, I'd drop the "list of" here, what is important is that it's records being compared, and it being a list isn't important to the test. |
||
var list = [(1, 'a'), (1, 'a'), (2, 'b'), (1, 'a')]; | ||
var frequencyMap = list.frequencies; | ||
expect(frequencyMap, {(1, 'a'): 3, (2, 'b'): 1}); | ||
}); | ||
|
||
test('list with elements that are objects', () { | ||
var list = [const MyObject(1), const MyObject(1), const MyObject(2)]; | ||
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. Don't use |
||
var frequencyMap = list.frequencies; | ||
expect(frequencyMap, {const MyObject(1): 2, const MyObject(2): 1}); | ||
}); | ||
|
||
test('list with equal numbers but different types', () { | ||
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. Good test. There is no way around web considering them all identical here, but different classes being equal is an important test for native numbers. (I was about to suggest testing with |
||
var list = [1, 1.0, 1, 1.0]; | ||
var frequencyMap = list.frequencies; | ||
expect(frequencyMap, {1: 4,}); | ||
}); | ||
|
||
test('list with mixed data types', () { | ||
var list = [1, 'one', true, 1, 'one', false]; | ||
var frequencyMap = list.frequencies; | ||
expect(frequencyMap, {1: 2, 'one': 2, true: 1, false: 1}); | ||
}); | ||
|
||
test('list with null values', () { | ||
var list = [null, null, 1, 'null']; | ||
var frequencyMap = list.frequencies; | ||
expect(frequencyMap, {null: 2, 1: 1, 'null': 1}); | ||
}); | ||
}); | ||
}); | ||
|
||
group('Comparator', () { | ||
|
@@ -2046,6 +2163,18 @@ void main() { | |
}); | ||
} | ||
|
||
class MyObject { | ||
final int id; | ||
const MyObject(this.id); | ||
|
||
@override | ||
bool operator ==(Object other) => | ||
identical(this, other) || other is MyObject && id == other.id; | ||
|
||
@override | ||
int get hashCode => id.hashCode; | ||
} | ||
|
||
/// Creates a plain iterable not implementing any other class. | ||
Iterable<T> iterable<T>(Iterable<T> values) sync* { | ||
yield* values; | ||
|
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.
Not a good example, though, since list equality is identity. Same below ...