Add toUnorderedList to PriorityQueue.
#152
Conversation
Also change `HeapPriorityQueue` to use `==` for `contains` and `remove` checks. Fix a bad null-safety migration for HeapPriorityQueue, and add tests to ensure it works with `null` values.
lib/src/priority_queue.dart
Outdated
| @@ -9,6 +9,18 @@ import 'utils.dart'; | |||
| /// A priority queue is a priority based work-list of elements. | |||
| /// | |||
| /// The queue allows adding elements, and removing them again in priority order. | |||
| /// The same object can be added to the queue more than once. | |||
| /// There is no specified oredering for objects with the same priority | |||
There was a problem hiding this comment.
| /// There is no specified oredering for objects with the same priority | |
| /// There is no specified ordering for objects with the same priority |
| } | ||
| return set; | ||
| } | ||
|
|
||
| @override | ||
| List<E> toUnorderedList() => _toUnorderedList(); |
There was a problem hiding this comment.
Why do we need the private one? Can we put the implementation here and skip the function?
There was a problem hiding this comment.
I call this method from toList too. It's an implementation detail that the two methods share code, so I don't want to risk someone overriding toUnorderedList and also affecting toList.
Generally, I avoid calling an overridable method from another method unless the latter method is explicitly documented as depending on the former. Code sharing is an implementation detail, so it shouldn't affect the visible API.
|
All our other collections accept There is no real reason to not to accept The |
|
@lrhn ready to merge? |
| if (comp == 0) return index; | ||
| if (comp < 0) { | ||
| if (comp <= 0) { | ||
| if (comp == 0 && element == object) return index; |
There was a problem hiding this comment.
This turned out to break at least one project's tests.
I think it's the right thing to do, and they were able to work around it.
There was a problem hiding this comment.
Super. If you rely on compareTo for equality, and don't override == to match, then I guess this would affect you.
Add `toUnorderedList` to `PriorityQueue`. Also change `HeapPriorityQueue` to use `==` for `contains` and `remove` checks. Fix a bad null-safety migration for HeapPriorityQueue, and add tests to ensure it works with `null` values.
Also change
HeapPriorityQueueto use==forcontainsandremovechecks.Fix a bad null-safety migration for HeapPriorityQueue, and add tests to ensure it works with
nullvalues.Fixes dart-lang/core#710, dart-lang/core#606