[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=10959

Iain Buclaw  changed:

   What|Removed |Added

   Priority|P2  |P3

--

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=10959

Seb  changed:

   What|Removed |Added

 CC||greensunn...@gmail.com

--- Comment #14 from Seb  ---
A PR to fix the landmines - https://github.com/dlang/phobos/pull/6154

It's probably too late to rename `remove` :/

--

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

https://d.puremagic.com/issues/show_bug.cgi?id=10959


Andrea Fontana advm...@katamail.com changed:

   What|Removed |Added

 CC||advm...@katamail.com


--- Comment #11 from Andrea Fontana advm...@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: ---

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

https://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #12 from daniel...@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: ---

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

https://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #8 from bearophile_h...@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: ---

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

https://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #9 from daniel...@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: ---

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

https://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #10 from monarchdo...@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: ---

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

https://d.puremagic.com/issues/show_bug.cgi?id=10959


Damian damian...@hotmail.co.uk changed:

   What|Removed |Added

 CC||damian...@hotmail.co.uk


--- Comment #7 from Damian damian...@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: ---

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

http://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #3 from bearophile_h...@eml.cc 2013-09-10 05:59:32 PDT ---
(In reply to comment #2)
 -1, I disagree on all points except to rename to `std.algorithm.removeAt` and
 add a complimentary method which instead removes by value.

Are you saying that std.algorithm.remove is not very bug-prone? And why?

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

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

http://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #4 from daniel...@bigpond.com 2013-09-10 07:28:02 PDT ---
After 24 hours of thinking about it, I've come to agree with your statement. My
original sentiment was that of likening std.algorithm.remove to its look-alike
http://www.cplusplus.com/reference/algorithm/remove.
I also saw the slice level purity of the operation as being an attractive
quality, however, given the majority of the range interfaces in D are mutating
by default, I see no reason why the behavior of this function should be
different.

To which end, I now agree on all points. Sorry, I'll hope you forgive me.

+1.

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

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

http://d.puremagic.com/issues/show_bug.cgi?id=10959



--- Comment #5 from bearophile_h...@eml.cc 2013-09-10 07:34:20 PDT ---
(In reply to comment #4)

 Sorry, I'll hope you forgive me.

Thank you, but you don't need to ask for forgiveness for just disagreeing with
me :-) Disagreeing is a natural part of discussions.

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

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

http://d.puremagic.com/issues/show_bug.cgi?id=10959


daniel...@bigpond.com changed:

   What|Removed |Added

 CC||daniel...@bigpond.com


--- Comment #2 from daniel...@bigpond.com 2013-09-09 17:15:16 PDT ---
-1, I disagree on all points except to rename to `std.algorithm.removeAt` and
add a complimentary method which instead removes by value.

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

[Issue 10959] std.algorithm.remove is highly bug-prone

[d-bugmail]

http://d.puremagic.com/issues/show_bug.cgi?id=10959


Dylan tcdknut...@gmail.com changed:

   What|Removed |Added

 CC||tcdknut...@gmail.com


--- Comment #1 from Dylan tcdknut...@gmail.com 2013-09-03 14:52:30 PDT ---
+1, regardless of impact on backwards compatibility. It's a landmine, and it's
bit me a few times in the past too.

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