phobos
phobos copied to clipboard
dlang
Add consume to SList
A range that iterates then removes elements for which the predicate is
true. An efficient combination of filter!(pred) and remove.
Thanks for your pull request, @thewilsonator! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Some tips to help speed things up:
-
smaller, focused PRs are easier to review than big ones
-
try not to mix up refactoring or style changes with bug fixes or feature enhancements
-
provide helpful commit messages explaining the rationale behind each change
Bear in mind that large or tricky changes may require multiple rounds of review and revision.
Please see CONTRIBUTING.md for more information.
Bugzilla references
Your PR doesn't reference any Bugzilla issue.
If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
This is a good idea, but we could achieve the desired behaviour by using std.algorithm.iteration.each
One could do something like this:
auto a = SList!int(-1, 1, -2, 1, -3, 4);
SList!int acc;
each!((int e) => (e > 0) ? acc.insert(e) : 0)(a[]);
I believe that consume should return a SList rather than a different type.
@andralex
That works but creates a new list (with allocations and copying of data), rather than mutate the old (reference semantics). The whole point is to amortise theO(n) iteration and provide an efficient way to remove based on a predicate.
Perhaps it would be more aptly named removeYield.
One could do something like this:
If you already create a new List, you can simply use filter, e.g.:
auto a = SList!int(-1, 1, -2, 1, -3, 4);
auto acc = a[].filter!(x => x > 0).SList!int;
acc.array.writeln;
Perhaps it would be more aptly named removeYield.
Yes, but the word yield isn't very common in D as everything is a range. removePred or remove might be better.
The whole point is to amortise theO(n) iteration and provide an efficient way to remove based on a predicate.
I get your point, but I'm not fully convinced that this is a popular operation, especially considering that std.container will hopefully get replaced / deprecated soonish. What do other people think? CC @ZombineDev @JackStouffer
This seems like a lot of code for a very specific problem. What's the use case?
This is a difficult and subtle API, though I see its necessity.
A tricky problem with SList is that removal primitives need to access the node just before the element being removed. I think the right approach is along the lines of linearRemoveElement. Currently the function takes a value and returns bool. A more composable version would return a range positioned just before the removed element. Then successive calls to linearRemoveElement would remove as many as needed, one at a time.
Then a predicated version of the function would remove by predicate, not a specific value.
Heh, completely forgot about it. It looks like removeLinearElement was added, but takes a value only not a predicate.
How about adding an overload that takes a predicate? I think that's what @andralex was suggesting