From ed3824a220e5405a8b6b68dbabc9dcc1d54efef7 Mon Sep 17 00:00:00 2001 From: "Lasse R.H. Nielsen" Date: Tue, 31 Aug 2021 12:44:39 +0000 Subject: [PATCH] Tweaks, refactoring and updates on DoubleLinkedQueue. It's a badly designed class, but this change makes it slightly better. (We should consider moving it to package:collection and removing it from the platform libraries. Eventually.) The changes decouples the public `DoubleLinkedQueueEntry` class from the actual queue implementation classes, avoiding the latter having to share a generic `_Link` interface which would force them to do otherwise unnecessary casts. The public class should have been abstract, but isn't. It's mainly used as the API of the entries exposed by DoubleLinkedQueue, but it can also be used by itself to build double-linked lists that are not part of a Queue, and the class can be extended. If anyone does either, it should keep working, so it needs to be a concrete, self-contained class with a generative constructor. That class is moved to dart:_internal to avoid its private implementation fields from interfering with the queue implementation classes, which have similar fields, but can now have them at different types. That's a hack, but it works. The internal implementation of the DoubleLinkedQueue and its entries are updated to better follow current programming idioms, and avoids having fields on classes which don't need them. Also fixes #27920 (`clear`ing a list now detaches each entry from the list so later `.contains` won't give the wrong answer). And moves queue tests from tests/corelib to tests/lib/collection, since Queue isn't exposed by `dart:core`. BUG= https://github.com/dart-lang/sdk/issues/27920 Change-Id: Ia3bb340a75886de160cc0e449947e8e7ee587061 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/211265 Commit-Queue: Lasse R.H. Nielsen Reviewed-by: Nate Bosch --- .../tests/concurrency/stress_test_list.json | 20 +- sdk/lib/collection/collection.dart | 2 + sdk/lib/collection/queue.dart | 332 ++++++++---------- sdk/lib/internal/internal.dart | 63 ++++ .../collection}/queue_first_test.dart | 5 +- .../collection}/queue_iterator_test.dart | 0 .../collection}/queue_last_test.dart | 5 +- .../collection}/queue_single_test.dart | 5 +- .../collection}/queue_test.dart | 63 ++-- .../collection}/queue_first_test.dart | 5 +- .../collection}/queue_iterator_test.dart | 0 .../collection}/queue_last_test.dart | 5 +- .../collection}/queue_single_test.dart | 5 +- .../collection}/queue_test.dart | 65 ++-- 14 files changed, 311 insertions(+), 264 deletions(-) rename tests/{corelib => lib/collection}/queue_first_test.dart (90%) rename tests/{corelib => lib/collection}/queue_iterator_test.dart (100%) rename tests/{corelib => lib/collection}/queue_last_test.dart (90%) rename tests/{corelib => lib/collection}/queue_single_test.dart (91%) rename tests/{corelib => lib/collection}/queue_test.dart (89%) rename tests/{corelib_2 => lib_2/collection}/queue_first_test.dart (90%) rename tests/{corelib_2 => lib_2/collection}/queue_iterator_test.dart (100%) rename tests/{corelib_2 => lib_2/collection}/queue_last_test.dart (90%) rename tests/{corelib_2 => lib_2/collection}/queue_single_test.dart (92%) rename tests/{corelib_2 => lib_2/collection}/queue_test.dart (90%) diff --git a/runtime/tests/concurrency/stress_test_list.json b/runtime/tests/concurrency/stress_test_list.json index 869c23330e0..80ae9a90311 100644 --- a/runtime/tests/concurrency/stress_test_list.json +++ b/runtime/tests/concurrency/stress_test_list.json @@ -321,11 +321,6 @@ "../../../tests/corelib/num_parse_test.dart", "../../../tests/corelib/num_sign_test.dart", "../../../tests/corelib/num_try_parse_test.dart", - "../../../tests/corelib/queue_first_test.dart", - "../../../tests/corelib/queue_iterator_test.dart", - "../../../tests/corelib/queue_last_test.dart", - "../../../tests/corelib/queue_single_test.dart", - "../../../tests/corelib/queue_test.dart", "../../../tests/corelib/range_error_test.dart", "../../../tests/corelib/reg_exp1_test.dart", "../../../tests/corelib/reg_exp5_test.dart", @@ -3103,6 +3098,11 @@ "../../../tests/lib/collection/hash_set_test.dart", "../../../tests/lib/collection/linked_list_test.dart", "../../../tests/lib/collection/list_test.dart", + "../../../tests/lib/collection/queue_first_test.dart", + "../../../tests/lib/collection/queue_iterator_test.dart", + "../../../tests/lib/collection/queue_last_test.dart", + "../../../tests/lib/collection/queue_single_test.dart", + "../../../tests/lib/collection/queue_test.dart", "../../../tests/lib/convert/ascii_test.dart", "../../../tests/lib/convert/base64_test.dart", "../../../tests/lib/convert/chunked_conversion1_test.dart", @@ -3734,11 +3734,6 @@ "../../../tests/corelib_2/num_parse_test.dart", "../../../tests/corelib_2/num_sign_test.dart", "../../../tests/corelib_2/num_try_parse_test.dart", - "../../../tests/corelib_2/queue_first_test.dart", - "../../../tests/corelib_2/queue_iterator_test.dart", - "../../../tests/corelib_2/queue_last_test.dart", - "../../../tests/corelib_2/queue_single_test.dart", - "../../../tests/corelib_2/queue_test.dart", "../../../tests/corelib_2/range_error_test.dart", "../../../tests/corelib_2/reg_exp1_test.dart", "../../../tests/corelib_2/reg_exp4_test.dart", @@ -6443,6 +6438,11 @@ "../../../tests/lib_2/collection/hash_set_test.dart", "../../../tests/lib_2/collection/linked_list_test.dart", "../../../tests/lib_2/collection/list_test.dart", + "../../../tests/lib_2/collection/queue_first_test.dart", + "../../../tests/lib_2/collection/queue_iterator_test.dart", + "../../../tests/lib_2/collection/queue_last_test.dart", + "../../../tests/lib_2/collection/queue_single_test.dart", + "../../../tests/lib_2/collection/queue_test.dart", "../../../tests/lib_2/convert/ascii_test.dart", "../../../tests/lib_2/convert/base64_test.dart", "../../../tests/lib_2/convert/chunked_conversion1_test.dart", diff --git a/sdk/lib/collection/collection.dart b/sdk/lib/collection/collection.dart index ce3d87f2917..ac592bd1b9a 100644 --- a/sdk/lib/collection/collection.dart +++ b/sdk/lib/collection/collection.dart @@ -14,6 +14,8 @@ library dart.collection; import 'dart:_internal' hide Symbol; import 'dart:math' show Random; // Used by ListMixin.shuffle. +export 'dart:_internal' show DoubleLinkedQueueEntry; + part 'collections.dart'; part 'hash_map.dart'; part 'hash_set.dart'; diff --git a/sdk/lib/collection/queue.dart b/sdk/lib/collection/queue.dart index fd83b42d097..dcdecb48871 100644 --- a/sdk/lib/collection/queue.dart +++ b/sdk/lib/collection/queue.dart @@ -119,116 +119,71 @@ abstract class Queue implements EfficientLengthIterable { void clear(); } -class _DoubleLink> { - Link? _previousLink; - Link? _nextLink; - - void _link(Link? previous, Link? next) { - _nextLink = next; - _previousLink = previous; - if (previous != null) previous._nextLink = this as Link; - if (next != null) next._previousLink = this as Link; - } - - void _unlink() { - if (_previousLink != null) _previousLink!._nextLink = _nextLink; - if (_nextLink != null) _nextLink!._previousLink = _previousLink; - _nextLink = null; - _previousLink = null; - } -} - -/// An entry in a doubly linked list. It contains a pointer to the next -/// entry, the previous entry, and the boxed element. -class DoubleLinkedQueueEntry extends _DoubleLink> { - // TODO(rnystrom): This needs to be nullable because the subclass - // _DoubleLinkedQueueSentinel does not have an element. A cleaner solution is - // probably to refactor the class hierarchy so that _DoubleLinkedQueueSentinel - // does not inherit an element field. - E? _element; - - /// The element in the queue. - E get element => _element as E; - set element(E element) { - _element = element; - } - - DoubleLinkedQueueEntry(this._element); - - /// Appends the given [e] as entry just after this entry. - void append(E e) { - DoubleLinkedQueueEntry(e)._link(this, _nextLink); - } - - /// Prepends the given [e] as entry just before this entry. - void prepend(E e) { - DoubleLinkedQueueEntry(e)._link(_previousLink, this); - } - - E remove() { - _unlink(); - return element; - } - - /// Returns the previous entry or `null` if there is none. - DoubleLinkedQueueEntry? previousEntry() => _previousLink; - - /// Returns the next entry or `null` if there is none. - DoubleLinkedQueueEntry? nextEntry() => _nextLink; -} - -/// Interface for the link classes used by [DoubleLinkedQueue]. +/// Interface and base class for the link classes used by [DoubleLinkedQueue]. /// /// Both the [_DoubleLinkedQueueElement] and [_DoubleLinkedQueueSentinel] /// implement this interface. -/// The entry contains a link back to the queue, so calling `append` -/// or `prepend` can correctly update the element count. -abstract class _DoubleLinkedQueueEntry extends DoubleLinkedQueueEntry { - DoubleLinkedQueue? _queue; - _DoubleLinkedQueueEntry(E? element, this._queue) : super(element); +abstract class _DoubleLinkedQueueEntry { + _DoubleLinkedQueueEntry? _previousLink; + _DoubleLinkedQueueEntry? _nextLink; - DoubleLinkedQueueEntry? _asNonSentinelEntry(); - - void _append(E e) { - _DoubleLinkedQueueElement(e, _queue)._link(this, _nextLink); + void _link( + _DoubleLinkedQueueEntry? previous, _DoubleLinkedQueueEntry? next) { + _nextLink = next; + _previousLink = previous; + previous?._nextLink = this; + next?._previousLink = this; } - void _prepend(E e) { - _DoubleLinkedQueueElement(e, _queue)._link(_previousLink, this); + void _unlink() { + _previousLink?._nextLink = _nextLink; + _nextLink?._previousLink = _previousLink; + _previousLink = _nextLink = null; + } + + _DoubleLinkedQueueElement? _asNonSentinelEntry(); + + void _append(E element, DoubleLinkedQueue? queue) { + _DoubleLinkedQueueElement(element, queue)._link(this, _nextLink); + } + + void _prepend(E element, DoubleLinkedQueue? queue) { + _DoubleLinkedQueueElement(element, queue)._link(_previousLink, this); } E _remove(); - E get _element => super._element as E; - - DoubleLinkedQueueEntry? nextEntry() { - _DoubleLinkedQueueEntry entry = _nextLink as _DoubleLinkedQueueEntry; - return entry._asNonSentinelEntry(); - } - - DoubleLinkedQueueEntry? previousEntry() { - _DoubleLinkedQueueEntry entry = - _previousLink as _DoubleLinkedQueueEntry; - return entry._asNonSentinelEntry(); - } + E get element; } -/// The actual entry type used by the [DoubleLinkedQueue]. +/// Linked list entry used by the [DoubleLinkedQueue] to hold an element. /// -/// The entry contains a reference to the queue, allowing +/// These entry objects are also exposed by [DoubleLinkedQueue.firstEntry], +/// [DoubleLinkedQueue.lastEntry] and [DoubleLinkedQueue.forEachEntry]. +/// +/// The entry contains both the [element] (which is mutable to anyone with +/// access to the entry object) and a reference to the queue, allowing /// [append]/[prepend] to update the list length. -class _DoubleLinkedQueueElement extends _DoubleLinkedQueueEntry { - _DoubleLinkedQueueElement(E element, DoubleLinkedQueue? queue) - : super(element, queue); +/// +/// When an entry is removed from its queue, the [_queue] is set to `null` +/// and will never change again. You can still use the unlinked entry +/// to create a new list, by calling [append] and [prepend], but it won't +/// be part of any [DoubleLinkedQueue]. +class _DoubleLinkedQueueElement extends _DoubleLinkedQueueEntry + implements DoubleLinkedQueueEntry { + DoubleLinkedQueue? _queue; + E element; + + _DoubleLinkedQueueElement(this.element, this._queue); void append(E e) { - _append(e); - if (_queue != null) _queue!._elementCount++; + _append(e, _queue); + _queue?._elementCount++; } void prepend(E e) { - _prepend(e); - if (_queue != null) _queue!._elementCount++; + _prepend(e, _queue); + _queue?._elementCount++; } E _remove() { @@ -238,30 +193,43 @@ class _DoubleLinkedQueueElement extends _DoubleLinkedQueueEntry { } E remove() { - if (_queue != null) _queue!._elementCount--; + _queue?._elementCount--; return _remove(); } - _DoubleLinkedQueueElement? _asNonSentinelEntry() { - return this; - } + _DoubleLinkedQueueElement _asNonSentinelEntry() => this; + + DoubleLinkedQueueEntry? previousEntry() => + _previousLink?._asNonSentinelEntry(); + + DoubleLinkedQueueEntry? nextEntry() => _nextLink?._asNonSentinelEntry(); } -/// A sentinel in a double linked list is used to manipulate the list -/// at both ends. -/// A double linked list has exactly one sentinel, +/// A header object used to hold the two ends of a double linked queue. +/// +/// A [DoubleLinkedQueue] has exactly one sentinel, /// which is the only entry when the list is constructed. -/// Initially, a sentinel has its next and previous entry point to itself. -/// A sentinel does not box any user element. +/// +/// Initially, a sentinel has its next and previous entries point to itself. +/// Its next and previous links are never `null` after creation, and +/// the entries linked always form a circular structure with the next link +/// pointing to the first element of the queue, and the previous link +/// pointing to the last element of the queue, or both pointing to itself +/// again if the queue becomes empty. +/// +/// Implements [_DoubleLinkedQueueEntry._remove] and +/// [_DoubleLinkedQueueEntry.element] as throwing because +/// it makes it simple to implement members like [Queue.removeFirst] +/// or [Queue.first] as throwing on an empty queue. +/// +/// A sentinel does not contain any user element. class _DoubleLinkedQueueSentinel extends _DoubleLinkedQueueEntry { - _DoubleLinkedQueueSentinel(DoubleLinkedQueue queue) : super(null, queue) { + _DoubleLinkedQueueSentinel() { _previousLink = this; _nextLink = this; } - DoubleLinkedQueueEntry? _asNonSentinelEntry() { - return null; - } + Null _asNonSentinelEntry() => null; /// Hit by, e.g., [DoubleLinkedQueue.removeFirst] if the queue is empty. E _remove() { @@ -269,7 +237,7 @@ class _DoubleLinkedQueueSentinel extends _DoubleLinkedQueueEntry { } /// Hit by, e.g., [DoubleLinkedQueue.first] if the queue is empty. - E get _element { + E get element { throw IterableElementError.noElement(); } } @@ -278,8 +246,8 @@ class _DoubleLinkedQueueSentinel extends _DoubleLinkedQueueEntry { /// /// Allows constant time add, remove-at-ends and peek operations. class DoubleLinkedQueue extends Iterable implements Queue { - late _DoubleLinkedQueueSentinel _sentinel = - _DoubleLinkedQueueSentinel(this); + final _DoubleLinkedQueueSentinel _sentinel = + _DoubleLinkedQueueSentinel(); int _elementCount = 0; @@ -318,49 +286,48 @@ class DoubleLinkedQueue extends Iterable implements Queue { int get length => _elementCount; void addLast(E value) { - _sentinel._prepend(value); + _sentinel._prepend(value, this); _elementCount++; } void addFirst(E value) { - _sentinel._append(value); + _sentinel._append(value, this); _elementCount++; } void add(E value) { - _sentinel._prepend(value); + _sentinel._prepend(value, this); _elementCount++; } void addAll(Iterable iterable) { for (final E value in iterable) { - _sentinel._prepend(value); + _sentinel._prepend(value, this); _elementCount++; } } E removeLast() { - _DoubleLinkedQueueEntry lastEntry = - _sentinel._previousLink as _DoubleLinkedQueueEntry; - E result = lastEntry._remove(); + // Hits sentinel's `_remove` if queue is empty. + E result = _sentinel._previousLink!._remove(); _elementCount--; return result; } E removeFirst() { - _DoubleLinkedQueueEntry firstEntry = - _sentinel._nextLink as _DoubleLinkedQueueEntry; - E result = firstEntry._remove(); + // Hits sentinel's `_remove` if queue is empty. + E result = _sentinel._nextLink!._remove(); _elementCount--; return result; } bool remove(Object? o) { - _DoubleLinkedQueueEntry entry = - _sentinel._nextLink as _DoubleLinkedQueueEntry; - while (!identical(entry, _sentinel)) { - bool equals = (entry._element == o); - if (!identical(this, entry._queue)) { + _DoubleLinkedQueueEntry entry = _sentinel._nextLink!; + while (true) { + var elementEntry = entry._asNonSentinelEntry(); + if (elementEntry == null) return false; + bool equals = (elementEntry.element == o); + if (!identical(this, elementEntry._queue)) { // Entry must still be in the queue. throw ConcurrentModificationError(this); } @@ -369,26 +336,26 @@ class DoubleLinkedQueue extends Iterable implements Queue { _elementCount--; return true; } - entry = entry._nextLink as _DoubleLinkedQueueEntry; + entry = entry._nextLink!; } - return false; } void _filter(bool test(E element), bool removeMatching) { - _DoubleLinkedQueueEntry entry = - _sentinel._nextLink as _DoubleLinkedQueueEntry; - while (!identical(entry, _sentinel)) { - bool matches = test(entry._element); - if (!identical(this, entry._queue)) { + _DoubleLinkedQueueEntry entry = _sentinel._nextLink!; + while (true) { + var elementEntry = entry._asNonSentinelEntry(); + if (elementEntry == null) return; + bool matches = test(elementEntry.element); + if (!identical(this, elementEntry._queue)) { // Entry must still be in the queue. throw ConcurrentModificationError(this); } - DoubleLinkedQueueEntry next = entry._nextLink!; // Cannot be null. + var next = entry._nextLink!; // Cannot be null while entry is in queue. if (identical(removeMatching, matches)) { - entry._remove(); + elementEntry._remove(); _elementCount--; } - entry = next as _DoubleLinkedQueueEntry; + entry = next; } } @@ -400,22 +367,17 @@ class DoubleLinkedQueue extends Iterable implements Queue { _filter(test, false); } - E get first { - DoubleLinkedQueueEntry firstEntry = _sentinel._nextLink!; - return firstEntry._element as E; - } + // Hits sentinel's `get element` if no element in queue. + E get first => _sentinel._nextLink!.element; - E get last { - DoubleLinkedQueueEntry lastEntry = _sentinel._previousLink!; - return lastEntry._element as E; - } + // Hits sentinel's `get element` if no element in queue. + E get last => _sentinel._previousLink!.element; E get single { // Note that this throws correctly if the queue is empty // because reading the element of the sentinel throws. if (identical(_sentinel._nextLink, _sentinel._previousLink)) { - DoubleLinkedQueueEntry entry = _sentinel._nextLink!; - return entry._element as E; + return _sentinel._nextLink!.element; } throw IterableElementError.tooMany(); } @@ -430,9 +392,8 @@ class DoubleLinkedQueue extends Iterable implements Queue { /// The entry objects can also be accessed using [lastEntry], /// and they can be iterated using [DoubleLinkedQueueEntry.nextEntry] and /// [DoubleLinkedQueueEntry.previousEntry]. - DoubleLinkedQueueEntry? firstEntry() { - return _sentinel.nextEntry(); - } + DoubleLinkedQueueEntry? firstEntry() => + _sentinel._nextLink!._asNonSentinelEntry(); /// The entry object of the last element in the queue. /// @@ -444,15 +405,22 @@ class DoubleLinkedQueue extends Iterable implements Queue { /// The entry objects can also be accessed using [firstEntry], /// and they can be iterated using [DoubleLinkedQueueEntry.nextEntry] and /// [DoubleLinkedQueueEntry.previousEntry]. - DoubleLinkedQueueEntry? lastEntry() { - return _sentinel.previousEntry(); - } + DoubleLinkedQueueEntry? lastEntry() => + _sentinel._previousLink!._asNonSentinelEntry(); - bool get isEmpty { - return (identical(_sentinel._nextLink, _sentinel)); - } + bool get isEmpty => identical(_sentinel._nextLink, _sentinel); void clear() { + var cursor = _sentinel._nextLink!; + while (true) { + var entry = cursor._asNonSentinelEntry(); + if (entry == null) break; + cursor = cursor._nextLink!; + entry + .._nextLink = null + .._previousLink = null + .._queue = null; + } _sentinel._nextLink = _sentinel; _sentinel._previousLink = _sentinel; _elementCount = 0; @@ -475,57 +443,59 @@ class DoubleLinkedQueue extends Iterable implements Queue { /// inserted after the current element before it is removed will not be /// visited by the iteration. void forEachEntry(void action(DoubleLinkedQueueEntry element)) { - _DoubleLinkedQueueEntry entry = - _sentinel._nextLink as _DoubleLinkedQueueEntry; - while (!identical(entry, _sentinel)) { - _DoubleLinkedQueueElement element = - entry as _DoubleLinkedQueueElement; - _DoubleLinkedQueueEntry next = - element._nextLink as _DoubleLinkedQueueEntry; - // Remember both entry and entry._nextLink. - // If someone calls `element.remove()` we continue from `next`. - // Otherwise we use the value of entry._nextLink which may have been - // updated. - action(element); - if (identical(this, entry._queue)) { - next = entry._nextLink as _DoubleLinkedQueueEntry; - } else if (!identical(this, next._queue)) { + var cursor = _sentinel._nextLink!; + while (true) { + var element = cursor._asNonSentinelEntry(); + if (element == null) break; + if (!identical(element._queue, this)) { throw ConcurrentModificationError(this); } - entry = next; + cursor = cursor._nextLink!; + // Remember both element and element._nextLink (as cursor). + // If someone calls `element.remove()` we continue from `next`. + // Otherwise we use the value of element._nextLink which may have been + // updated. + action(element); + if (identical(this, element._queue)) { + cursor = element._nextLink!; + } } } _DoubleLinkedQueueIterator get iterator { - return _DoubleLinkedQueueIterator(_sentinel); + return _DoubleLinkedQueueIterator(this); } String toString() => IterableBase.iterableToFullString(this, '{', '}'); } class _DoubleLinkedQueueIterator implements Iterator { - _DoubleLinkedQueueSentinel? _sentinel; - DoubleLinkedQueueEntry? _nextEntry; + /// Queue being iterated. Used for concurrent modification checks. + DoubleLinkedQueue? _queue; + + /// Next entry to visit. Set to null when hitting the sentinel. + _DoubleLinkedQueueEntry? _nextEntry; + + /// Current element value, when valid. E? _current; - _DoubleLinkedQueueIterator(_DoubleLinkedQueueSentinel sentinel) - : _sentinel = sentinel, - _nextEntry = sentinel._nextLink; + _DoubleLinkedQueueIterator(DoubleLinkedQueue this._queue) + : _nextEntry = _queue._sentinel._nextLink; bool moveNext() { - if (identical(_nextEntry, _sentinel)) { + var nextElement = _nextEntry?._asNonSentinelEntry(); + if (nextElement == null) { + // Clear everything to not unnecessarily keep values alive. _current = null; _nextEntry = null; - _sentinel = null; + _queue = null; return false; } - _DoubleLinkedQueueEntry elementEntry = - _nextEntry as _DoubleLinkedQueueEntry; - if (!identical(_sentinel!._queue, elementEntry._queue)) { - throw ConcurrentModificationError(_sentinel!._queue); + if (!identical(_queue, nextElement._queue)) { + throw ConcurrentModificationError(_queue); } - _current = elementEntry._element; - _nextEntry = elementEntry._nextLink; + _current = nextElement.element; + _nextEntry = nextElement._nextLink; return true; } diff --git a/sdk/lib/internal/internal.dart b/sdk/lib/internal/internal.dart index 0e4c8983359..71f02d92e47 100644 --- a/sdk/lib/internal/internal.dart +++ b/sdk/lib/internal/internal.dart @@ -948,3 +948,66 @@ abstract class HttpStatus { @Deprecated("Use networkConnectTimeoutError instead") static const int NETWORK_CONNECT_TIMEOUT_ERROR = networkConnectTimeoutError; } + +// Class moved here from dart:collection +// to allow another, more important, class to implement the interface +// without having to match the private members. + +/// An entry in a doubly linked list. +/// +/// Such an entry contains an element and a link to the previous or next +/// entries, if any. +// +// This class should have been abstract, but originally wasn't. +// It's not used itself to interact with the double linked queue class, +// which uses the `_DoubleLinkedQueueEntry` class and subclasses instead. +// It's only used as an interface for the +// `DoubleLinkedQueue.forEach`, `DoubleLinkedQueue.firstEntry` and +// `DoubleLinkedQueue.lastEntry` members. +// Still, someone might have based their own double-linked list on this +// class, so we keep it functional. +class DoubleLinkedQueueEntry { + DoubleLinkedQueueEntry? _previousLink; + DoubleLinkedQueueEntry? _nextLink; + + /// The element of the entry in the queue. + E element; + + /// Creates a new entry with the given [element]. + DoubleLinkedQueueEntry(this.element); + + void _link( + DoubleLinkedQueueEntry? previous, DoubleLinkedQueueEntry? next) { + _nextLink = next; + _previousLink = previous; + previous?._nextLink = this; + next?._previousLink = this; + } + + /// Appends the given element [e] as entry just after this entry. + void append(E e) { + DoubleLinkedQueueEntry(e)._link(this, _nextLink); + } + + /// Prepends the given [e] as entry just before this entry. + void prepend(E e) { + DoubleLinkedQueueEntry(e)._link(_previousLink, this); + } + + /// Removes this entry from any chain of entries it is part of. + /// + /// Returns its element value. + E remove() { + _previousLink?._nextLink = _nextLink; + _nextLink?._previousLink = _previousLink; + _nextLink = null; + _previousLink = null; + return element; + } + + /// The previous entry, or `null` if there is none. + DoubleLinkedQueueEntry? previousEntry() => _previousLink; + + /// The next entry, or `null` if there is none. + DoubleLinkedQueueEntry? nextEntry() => _nextLink; +} diff --git a/tests/corelib/queue_first_test.dart b/tests/lib/collection/queue_first_test.dart similarity index 90% rename from tests/corelib/queue_first_test.dart rename to tests/lib/collection/queue_first_test.dart index d0baf997b62..fb7e1a63a34 100644 --- a/tests/corelib/queue_first_test.dart +++ b/tests/lib/collection/queue_first_test.dart @@ -9,7 +9,10 @@ import 'dart:collection' show Queue; main() { Queue queue1 = new Queue(); - queue1..add(11)..add(12)..add(13); + queue1 + ..add(11) + ..add(12) + ..add(13); Queue queue2 = new Queue(); Expect.equals(11, queue1.first); diff --git a/tests/corelib/queue_iterator_test.dart b/tests/lib/collection/queue_iterator_test.dart similarity index 100% rename from tests/corelib/queue_iterator_test.dart rename to tests/lib/collection/queue_iterator_test.dart diff --git a/tests/corelib/queue_last_test.dart b/tests/lib/collection/queue_last_test.dart similarity index 90% rename from tests/corelib/queue_last_test.dart rename to tests/lib/collection/queue_last_test.dart index 3ea2eff0af7..288ad4070cb 100644 --- a/tests/corelib/queue_last_test.dart +++ b/tests/lib/collection/queue_last_test.dart @@ -9,7 +9,10 @@ import 'dart:collection' show Queue; main() { Queue queue1 = new Queue(); - queue1..add(11)..add(12)..add(13); + queue1 + ..add(11) + ..add(12) + ..add(13); Queue queue2 = new Queue(); Expect.equals(13, queue1.last); diff --git a/tests/corelib/queue_single_test.dart b/tests/lib/collection/queue_single_test.dart similarity index 91% rename from tests/corelib/queue_single_test.dart rename to tests/lib/collection/queue_single_test.dart index 1ef6b59bd13..a4cdfbf4fc0 100644 --- a/tests/corelib/queue_single_test.dart +++ b/tests/lib/collection/queue_single_test.dart @@ -11,7 +11,10 @@ main() { Queue queue1 = new Queue(); queue1.add(42); Queue queue2 = new Queue(); - queue2..add(11)..add(12)..add(13); + queue2 + ..add(11) + ..add(12) + ..add(13); Queue queue3 = new Queue(); Expect.equals(42, queue1.single); diff --git a/tests/corelib/queue_test.dart b/tests/lib/collection/queue_test.dart similarity index 89% rename from tests/corelib/queue_test.dart rename to tests/lib/collection/queue_test.dart index a34038e59d8..0abfa511da3 100644 --- a/tests/corelib/queue_test.dart +++ b/tests/lib/collection/queue_test.dart @@ -56,41 +56,29 @@ abstract class QueueTest { Queue other = newQueueFrom(queue.where(is10)); checkQueue(other, 1, 10); - Expect.equals(true, queue.any(is10)); + Expect.isTrue(queue.any(is10)); bool isInstanceOfInt(dynamic value) { return (value is int); } - Expect.equals(true, queue.every(isInstanceOfInt)); + Expect.isTrue(queue.every(isInstanceOfInt)); - Expect.equals(false, queue.every(is10)); + Expect.isFalse(queue.every(is10)); bool is1(dynamic value) { return (value == 1); } - Expect.equals(false, queue.any(is1)); + Expect.isFalse(queue.any(is1)); queue.clear(); Expect.equals(0, queue.length); - var exception = null; - try { - queue.removeFirst(); - } on StateError catch (e) { - exception = e; - } - Expect.equals(true, exception != null); + Expect.throws(queue.removeFirst); Expect.equals(0, queue.length); - exception = null; - try { - queue.removeLast(); - } on StateError catch (e) { - exception = e; - } - Expect.equals(true, exception != null); + Expect.throws(queue.removeLast); Expect.equals(0, queue.length); queue.addFirst(1); @@ -134,7 +122,7 @@ abstract class QueueTest { Expect.equals(expectedSum, sum); } - testLength(int length, Queue queue) { + void testLength(int length, Queue queue) { Expect.equals(length, queue.length); ((length == 0) ? Expect.isTrue : Expect.isFalse)(queue.isEmpty); ((length != 0) ? Expect.isTrue : Expect.isFalse)(queue.isNotEmpty); @@ -242,9 +230,12 @@ abstract class QueueTest { queue.toList()); // Regression test: http://dartbug.com/16270 - // This should do nothing, and should not throw. + // These should all do nothing, and should not throw. Queue emptyQueue = newQueue(); emptyQueue.remove(0); + emptyQueue.removeWhere((x) => throw "unreachable"); + emptyQueue.retainWhere((x) => throw "unreachable"); + Expect.equals(0, emptyQueue.length); } void testLarge() { @@ -414,11 +405,11 @@ class DoubleLinkedQueueTest extends QueueTest { DoubleLinkedQueueEntry? entry1 = queue1.firstEntry(); DoubleLinkedQueueEntry? entry2 = queue2.firstEntry(); while (entry1 != null) { - Expect.equals(true, !identical(entry1, entry2)); + Expect.notIdentical(entry1, entry2); entry1 = entry1.nextEntry(); entry2 = entry2!.nextEntry(); } - Expect.equals(null, entry2); + Expect.isNull(entry2); var firstEntry = queue1.firstEntry()!; var secondEntry = queue1.firstEntry()!.nextEntry()!; @@ -430,19 +421,27 @@ class DoubleLinkedQueueTest extends QueueTest { thirdEntry.prepend(8); thirdEntry.append(9); Expect.equals(9, queue1.length); - Expect.listEquals(queue1.toList(), [4, 1, 5, 6, 2, 7, 8, 3, 9]); + Expect.listEquals([4, 1, 5, 6, 2, 7, 8, 3, 9], queue1.toList()); Expect.equals(1, firstEntry.remove()); Expect.equals(2, secondEntry.remove()); Expect.equals(3, thirdEntry.remove()); Expect.equals(6, queue1.length); - Expect.listEquals(queue1.toList(), [4, 5, 6, 7, 8, 9]); + Expect.listEquals([4, 5, 6, 7, 8, 9], queue1.toList()); + + var elements = []; + queue1.forEachEntry((entry) { + elements.add(entry.element); + entry.remove(); // Can remove while iterating. + }); + Expect.listEquals([4, 5, 6, 7, 8, 9], elements); + Expect.isTrue(queue1.isEmpty); } } void linkEntryTest() { var entry = new DoubleLinkedQueueEntry(42); - Expect.equals(null, entry.previousEntry()); - Expect.equals(null, entry.nextEntry()); + Expect.isNull(entry.previousEntry()); + Expect.isNull(entry.nextEntry()); entry.append(37); entry.prepend(87); @@ -453,8 +452,8 @@ void linkEntryTest() { Expect.equals(87, prev.element); Expect.identical(entry, prev.nextEntry()); Expect.identical(entry, next.previousEntry()); - Expect.equals(null, next.nextEntry()); - Expect.equals(null, prev.previousEntry()); + Expect.isNull(next.nextEntry()); + Expect.isNull(prev.previousEntry()); entry.element = 117; Expect.equals(117, entry.element); @@ -464,15 +463,15 @@ void linkEntryTest() { Expect.equals(117, entry.remove()); Expect.identical(next, prev.nextEntry()); Expect.identical(prev, next.previousEntry()); - Expect.equals(null, next.nextEntry()); - Expect.equals(null, prev.previousEntry()); + Expect.isNull(next.nextEntry()); + Expect.isNull(prev.previousEntry()); Expect.equals(37, next.element); Expect.equals(87, prev.element); Expect.equals(37, next.remove()); Expect.equals(87, prev.element); - Expect.equals(null, prev.nextEntry()); - Expect.equals(null, prev.previousEntry()); + Expect.isNull(prev.nextEntry()); + Expect.isNull(prev.previousEntry()); Expect.equals(87, prev.remove()); } diff --git a/tests/corelib_2/queue_first_test.dart b/tests/lib_2/collection/queue_first_test.dart similarity index 90% rename from tests/corelib_2/queue_first_test.dart rename to tests/lib_2/collection/queue_first_test.dart index 63ec6ca16f0..99fe1c6549f 100644 --- a/tests/corelib_2/queue_first_test.dart +++ b/tests/lib_2/collection/queue_first_test.dart @@ -11,7 +11,10 @@ import 'dart:collection' show Queue; main() { Queue queue1 = new Queue(); - queue1..add(11)..add(12)..add(13); + queue1 + ..add(11) + ..add(12) + ..add(13); Queue queue2 = new Queue(); Expect.equals(11, queue1.first); diff --git a/tests/corelib_2/queue_iterator_test.dart b/tests/lib_2/collection/queue_iterator_test.dart similarity index 100% rename from tests/corelib_2/queue_iterator_test.dart rename to tests/lib_2/collection/queue_iterator_test.dart diff --git a/tests/corelib_2/queue_last_test.dart b/tests/lib_2/collection/queue_last_test.dart similarity index 90% rename from tests/corelib_2/queue_last_test.dart rename to tests/lib_2/collection/queue_last_test.dart index 3d070dac761..f5575f1d478 100644 --- a/tests/corelib_2/queue_last_test.dart +++ b/tests/lib_2/collection/queue_last_test.dart @@ -11,7 +11,10 @@ import 'dart:collection' show Queue; main() { Queue queue1 = new Queue(); - queue1..add(11)..add(12)..add(13); + queue1 + ..add(11) + ..add(12) + ..add(13); Queue queue2 = new Queue(); Expect.equals(13, queue1.last); diff --git a/tests/corelib_2/queue_single_test.dart b/tests/lib_2/collection/queue_single_test.dart similarity index 92% rename from tests/corelib_2/queue_single_test.dart rename to tests/lib_2/collection/queue_single_test.dart index c1ebef11ff9..29654c6d292 100644 --- a/tests/corelib_2/queue_single_test.dart +++ b/tests/lib_2/collection/queue_single_test.dart @@ -13,7 +13,10 @@ main() { Queue queue1 = new Queue(); queue1.add(42); Queue queue2 = new Queue(); - queue2..add(11)..add(12)..add(13); + queue2 + ..add(11) + ..add(12) + ..add(13); Queue queue3 = new Queue(); Expect.equals(42, queue1.single); diff --git a/tests/corelib_2/queue_test.dart b/tests/lib_2/collection/queue_test.dart similarity index 90% rename from tests/corelib_2/queue_test.dart rename to tests/lib_2/collection/queue_test.dart index 08ea375f995..6a903e6cfd0 100644 --- a/tests/corelib_2/queue_test.dart +++ b/tests/lib_2/collection/queue_test.dart @@ -58,41 +58,29 @@ abstract class QueueTest { Queue other = newQueueFrom(queue.where(is10)); checkQueue(other, 1, 10); - Expect.equals(true, queue.any(is10)); + Expect.isTrue(queue.any(is10)); bool isInstanceOfInt(Object value) { return (value is int); } - Expect.equals(true, queue.every(isInstanceOfInt)); + Expect.isTrue(queue.every(isInstanceOfInt)); - Expect.equals(false, queue.every(is10)); + Expect.isFalse(queue.every(is10)); bool is1(Object value) { return (value == 1); } - Expect.equals(false, queue.any(is1)); + Expect.isFalse(queue.any(is1)); queue.clear(); Expect.equals(0, queue.length); - var exception = null; - try { - queue.removeFirst(); - } on StateError catch (e) { - exception = e; - } - Expect.equals(true, exception != null); + Expect.throws(queue.removeFirst); Expect.equals(0, queue.length); - exception = null; - try { - queue.removeLast(); - } on StateError catch (e) { - exception = e; - } - Expect.equals(true, exception != null); + Expect.throws(queue.removeLast); Expect.equals(0, queue.length); queue.addFirst(1); @@ -136,7 +124,7 @@ abstract class QueueTest { Expect.equals(expectedSum, sum); } - testLength(int length, Queue queue) { + void testLength(int length, Queue queue) { Expect.equals(length, queue.length); ((length == 0) ? Expect.isTrue : Expect.isFalse)(queue.isEmpty); ((length != 0) ? Expect.isTrue : Expect.isFalse)(queue.isNotEmpty); @@ -164,8 +152,6 @@ abstract class QueueTest { sum += e; } - ; - set.forEach(f); Expect.equals(7, sum); sum = 0; @@ -249,8 +235,9 @@ abstract class QueueTest { // These should all do nothing, and should not throw. Queue emptyQueue = newQueue(); emptyQueue.remove(0); - emptyQueue.removeWhere((x) => null); - emptyQueue.retainWhere((x) => null); + emptyQueue.removeWhere((x) => throw "unreachable"); + emptyQueue.retainWhere((x) => throw "unreachable"); + Expect.equals(0, emptyQueue.length); } void testLarge() { @@ -420,11 +407,11 @@ class DoubleLinkedQueueTest extends QueueTest { DoubleLinkedQueueEntry entry1 = queue1.firstEntry(); DoubleLinkedQueueEntry entry2 = queue2.firstEntry(); while (entry1 != null) { - Expect.equals(true, !identical(entry1, entry2)); + Expect.notIdentical(entry1, entry2); entry1 = entry1.nextEntry(); entry2 = entry2.nextEntry(); } - Expect.equals(null, entry2); + Expect.isNull(entry2); var firstEntry = queue1.firstEntry(); var secondEntry = queue1.firstEntry().nextEntry(); @@ -436,19 +423,27 @@ class DoubleLinkedQueueTest extends QueueTest { thirdEntry.prepend(8); thirdEntry.append(9); Expect.equals(9, queue1.length); - Expect.listEquals(queue1.toList(), [4, 1, 5, 6, 2, 7, 8, 3, 9]); + Expect.listEquals([4, 1, 5, 6, 2, 7, 8, 3, 9], queue1.toList()); Expect.equals(1, firstEntry.remove()); Expect.equals(2, secondEntry.remove()); Expect.equals(3, thirdEntry.remove()); Expect.equals(6, queue1.length); - Expect.listEquals(queue1.toList(), [4, 5, 6, 7, 8, 9]); + Expect.listEquals([4, 5, 6, 7, 8, 9], queue1.toList()); + + var elements = []; + queue1.forEachEntry((entry) { + elements.add(entry.element); + entry.remove(); // Can remove while iterating. + }); + Expect.listEquals([4, 5, 6, 7, 8, 9], elements); + Expect.isTrue(queue1.isEmpty); } } void linkEntryTest() { var entry = new DoubleLinkedQueueEntry(42); - Expect.equals(null, entry.previousEntry()); - Expect.equals(null, entry.nextEntry()); + Expect.isNull(entry.previousEntry()); + Expect.isNull(entry.nextEntry()); entry.append(37); entry.prepend(87); @@ -459,8 +454,8 @@ void linkEntryTest() { Expect.equals(87, prev.element); Expect.identical(entry, prev.nextEntry()); Expect.identical(entry, next.previousEntry()); - Expect.equals(null, next.nextEntry()); - Expect.equals(null, prev.previousEntry()); + Expect.isNull(next.nextEntry()); + Expect.isNull(prev.previousEntry()); entry.element = 117; Expect.equals(117, entry.element); @@ -470,15 +465,15 @@ void linkEntryTest() { Expect.equals(117, entry.remove()); Expect.identical(next, prev.nextEntry()); Expect.identical(prev, next.previousEntry()); - Expect.equals(null, next.nextEntry()); - Expect.equals(null, prev.previousEntry()); + Expect.isNull(next.nextEntry()); + Expect.isNull(prev.previousEntry()); Expect.equals(37, next.element); Expect.equals(87, prev.element); Expect.equals(37, next.remove()); Expect.equals(87, prev.element); - Expect.equals(null, prev.nextEntry()); - Expect.equals(null, prev.previousEntry()); + Expect.isNull(prev.nextEntry()); + Expect.isNull(prev.previousEntry()); Expect.equals(87, prev.remove()); }