Thread overview
[Issue 10959] std.algorithm.remove is highly bug-prone
Jan 09, 2014
Damian
Jan 23, 2014
Andrea Fontana
January 09, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #6 from bearophile_hugs@eml.cc 2014-01-09 03:11:59 PST ---
In Python to remove an item from an array (list) you use:

>>> items = [10, 20, 30, 20]
>>> items.remove(20)
>>> items
[10, 30, 20]


With D+Phobos you need:

void main() {
    import std.stdio, std.algorithm;
    auto items = [10, 20, 30, 20];
    items = items.remove(items.countUntil(20));
    items.writeln;
}


So this API is quite bad all around, it's not just bug-prone.

So I suggest to introduce a differently named function that works similarly to the current remove (but it doesn't need the bug-prone re-assignment on the left):

items.removeAtIndex(1);

And modify remove() to work like this, as the Python remove method:

items.remove(20);

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 09, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=10959


Damian <damianday@hotmail.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |damianday@hotmail.co.uk


--- Comment #7 from Damian <damianday@hotmail.co.uk> 2014-01-09 11:21:19 PST ---
I absolutely agree, I find this crops up all the time
when using containers, so I usually roll my own, which is a shame!

Bring on removeAtIndex!

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 22, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #8 from bearophile_hugs@eml.cc 2014-01-22 15:33:13 PST ---
> items = items.remove(items.countUntil(needle));

monarch_dodra comments:

> Hum... that requires iterating the range twice for a non-RA range. And you forgot a save:
> 
> items = items.remove(items.save.countUntil(needle));

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 23, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #9 from daniel350@bigpond.com 2014-01-22 21:33:11 PST ---
I've just given up on this idiom, instead using:

```
import std.algorithm;
import std.stdio;

void main() {
    auto items = [10, 20, 30];
    auto t = items.filter!(x => x != 20).copy(items);
    items = items[0 .. $ - t.length];

    writeln(items);
}
```

Its the only option that has clear time and space semantics.

Ref: http://dpaste.dzfl.pl/84273326

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 23, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #10 from monarchdodra@gmail.com 2014-01-22 23:22:14 PST ---
(In reply to comment #9)
> I've just given up on this idiom, instead using:
> 
> ```
> import std.algorithm;
> import std.stdio;
> 
> void main() {
>     auto items = [10, 20, 30];
>     auto t = items.filter!(x => x != 20).copy(items);
>     items = items[0 .. $ - t.length];
> 
>     writeln(items);
> }
> ```
> 
> Its the only option that has clear time and space semantics.
> 
> Ref: http://dpaste.dzfl.pl/84273326

How is that any different from:
items = items.remove!(x => x == 20)()
?

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 23, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=10959


Andrea Fontana <advmail@katamail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |advmail@katamail.com


--- Comment #11 from Andrea Fontana <advmail@katamail.com> 2014-01-23 00:55:42 PST ---
+1 for this bug. I've to double-check remove every single time I use it.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 23, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #12 from daniel350@bigpond.com 2014-01-23 01:52:09 PST ---
(In reply to comment #10)
> (In reply to comment #9)
> > I've just given up on this idiom, instead using:
> > 
> > ```
> > import std.algorithm;
> > import std.stdio;
> > 
> > void main() {
> >     auto items = [10, 20, 30];
> >     auto t = items.filter!(x => x != 20).copy(items);
> >     items = items[0 .. $ - t.length];
> > 
> >     writeln(items);
> > }
> > ```
> > 
> > Its the only option that has clear time and space semantics.
> > 
> > Ref: http://dpaste.dzfl.pl/84273326
> 
> How is that any different from:
> items = items.remove!(x => x == 20)()
> ?

A fair point, I'd forgotten about the predicate remove.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
March 16, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #13 from bearophile_hugs@eml.cc 2014-03-16 08:31:34 PDT ---
An extra feature of the new "removeIndex" (or "removeAtIndex") function is to optionally accept a range of indexes:

myArray = [.....];
myArray.removeIndex(2);
myArray.removeIndex(iota(1, 20, 3));

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------