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

           Summary: std.algorithm.remove is highly bug-prone
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody@puremagic.com
        ReportedBy: bearophile_hugs@eml.cc


--- Comment #0 from bearophile_hugs@eml.cc 2013-09-03 14:47:27 PDT ---
This is not an enhancement request, I consider this a bug report.


Often when I use std.algorithm.remove in my code I introduce bugs. So I believe std.algorithm.remove is too much bug-prone, some examples:

import std.algorithm: remove;
import std.stdio: writeln;
void main() {
    auto A = [1, 2, 3];
    A.remove(2);
    writeln(A); // prints: [1, 2, 3]
    A.remove!q{a = 2};
    writeln(A); // prints: [1, 2, 3]
    A.remove!q{a == 2};
    writeln(A); // prints: [1, 3, 3]
    A = [1, 2, 3];
    A = A.remove!q{a == 2};
    writeln(A); // prints: [1, 3] (correct)
}


So I suggest to rename std.algorithm.remove as "removeAtIndex" or something similar. And then to introduce a function remove that removes the given item. But this is not enough, because even this syntax is bug-prone to remove the item '2' from the array 'A':

A = A.remove(2);

What I should write is just:

A.remove(2);

This is how it's done in Python, and it's not bug-prone, this is how in my opinion it should be designed:

>>> A = [1, 2, 3]
>>> A.remove(2)
>>> A
[1, 3]


I don't care about all the discussions about containers, ranges, etc. Currently std.algorithm.remove is a landmine and in my opinion it's not acceptable in Phobos.

See also a thread by Manu and friends, that have had problems to remove an array item:

http://forum.dlang.org/thread/mailman.680.1378001151.1719.digitalmars-d@puremagic.com

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

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


Dylan <tcdknutson@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tcdknutson@gmail.com


--- Comment #1 from Dylan <tcdknutson@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: -------

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


daniel350@bigpond.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |daniel350@bigpond.com


--- Comment #2 from daniel350@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: -------

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



--- Comment #3 from bearophile_hugs@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: -------

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



--- Comment #4 from daniel350@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: -------

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



--- Comment #5 from bearophile_hugs@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: -------