www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 10959] New: std.algorithm.remove is highly bug-prone

reply d-bugmail puremagic.com writes:
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



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: -------
Sep 03 2013
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10959


Dylan <tcdknutson gmail.com> changed:

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



+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: -------
Sep 03 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10959


daniel350 bigpond.com changed:

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



-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: -------
Sep 09 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10959






 -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: -------
Sep 10 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10959




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: -------
Sep 10 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10959







 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: -------
Sep 10 2013