www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - std.algorithm's remove

reply maarten van damme <maartenvd1994 gmail.com> writes:
hello,

I'm a hobyist-programmer and around where I live there's a group of haskell
fanatics. They posted solutions to a recent programming challenge which I
find to be a bit ugly. For fun I wanted to implement it in d and a rough
version (not correct yet, this was written/hacked in 5 minutes after
reading the exercise)

My rough version is posted here : http://dpaste.dzfl.pl/4b5a6578

if you look at the output, you'll see this particular line :
"omkom -> komkom because of : kom momkom momkom -> momkomm"

This is because of what remove from std.algorithm does. It not only returns
a range with that element removed (as the name implies), it also modifies
the original range.
I assume this decision was made for efficiency purposes but that is one of

'ref' in it's parameters so you know something's up. Is there any way I
could've known this? (apart from reading the documentation on every single
trivial function in the std library?)
Aug 24 2013
next sibling parent reply "Brad Anderson" <eco gnuk.net> writes:
On Sunday, 25 August 2013 at 02:24:30 UTC, maarten van damme 
wrote:

 hello,

 I'm a hobyist-programmer and around where I live there's a 
 group of haskell
 fanatics. They posted solutions to a recent programming 
 challenge which I
 find to be a bit ugly. For fun I wanted to implement it in d 
 and a rough
 version (not correct yet, this was written/hacked in 5 minutes 
 after
 reading the exercise)

 My rough version is posted here : http://dpaste.dzfl.pl/4b5a6578

 if you look at the output, you'll see this particular line :
 "omkom -> komkom because of : kom momkom momkom -> momkomm"

 This is because of what remove from std.algorithm does. It not 
 only returns
 a range with that element removed (as the name implies), it 
 also modifies
 the original range.
 I assume this decision was made for efficiency purposes but 
 that is one of

 forces the
 'ref' in it's parameters so you know something's up. Is there 
 any way I
 could've known this? (apart from reading the documentation on 
 every single
 trivial function in the std library?)


It was done that way intentionally because the purpose of remove 
is to remove from the source range.  If you don't want to affect 
the source range use filter.

I suspect you could trace remove's lineage back to C++ STL's 
remove which works similarly (but is significantly clunkier and 
harder to use).
Aug 24 2013
next sibling parent reply maarten van damme <maartenvd1994 gmail.com> writes:
But remove doesn't truly remove from the source range because the length of
the source range stays the same. It's return value is a modified copy of
the source range.

Filter doesn't really work right away because that would also remove
duplicate elements while I only want to remove at a given index. It also
makes for clunkier looking code and is counterintinuitive to come up with
(I want to remove an element therefore I have to filter every element that
isn't equal to that element from the source range...)
And while I haven't worked in c++, even that appears to have remove_copy
which is really what I want.

I could use temporary variables and blow that neat line up in 3 lines
which, while still readable, look redundant.

I'm happy the d result is 3 times shorter then their haskell solution but
not that the time spent was 5 minutes working and the rest of the time
debugging. This as opposed to using racket where the time spent is 15
minutes but after that everything works great (while my racket experience
is limited to playing around with it for 2 weeks now and then).


2013/8/25 Brad Anderson <eco gnuk.net>


 On Sunday, 25 August 2013 at 02:24:30 UTC, maarten van damme wrote:


 hello,

 I'm a hobyist-programmer and around where I live there's a group of
 haskell
 fanatics. They posted solutions to a recent programming challenge which I
 find to be a bit ugly. For fun I wanted to implement it in d and a rough
 version (not correct yet, this was written/hacked in 5 minutes after
 reading the exercise)

 My rough version is posted here : http://dpaste.dzfl.pl/4b5a6578

 if you look at the output, you'll see this particular line :
 "omkom -> komkom because of : kom momkom momkom -> momkomm"

 This is because of what remove from std.algorithm does. It not only
 returns
 a range with that element removed (as the name implies), it also modifies
 the original range.
 I assume this decision was made for efficiency purposes but that is one of

 'ref' in it's parameters so you know something's up. Is there any way I
 could've known this? (apart from reading the documentation on every single
 trivial function in the std library?)


 It was done that way intentionally because the purpose of remove is to
 remove from the source range.  If you don't want to affect the source range
 use filter.

 I suspect you could trace remove's lineage back to C++ STL's remove which
 works similarly (but is significantly clunkier and harder to use).
Aug 24 2013
parent reply "bearophile" <bearophileHUGS lycos.com> writes:
maarten van damme:


 But remove doesn't truly remove from the source range because 
 the length of
 the source range stays the same. It's return value is a 
 modified copy of
 the source range.

 Filter doesn't really work right away because that would also 
 remove
 duplicate elements while I only want to remove at a given 
 index. It also
 makes for clunkier looking code and is counterintinuitive to 
 come up with
 (I want to remove an element therefore I have to filter every 
 element that
 isn't equal to that element from the source range...)
 And while I haven't worked in c++, even that appears to have 
 remove_copy
 which is really what I want.


I agree that the design of std.algorithm.remove is bug prone. The 
compiler doesn't verify that you are using its result! I have 
also filed several bugs regarding the implementation of 
std.algorithm.remove.

In the end your complaints are real. You can't expect Phobos to 
be perfect, but people work to fix it. Fixing such problems 
future D users will not find your problem. So I suggest you to 
think well about what your problem is, what you don't like on 
this, and then write a enhancement request for Phobos. Even if 
std.algorithm.remove can't be fixed, people can add a new 
function with a safer/more handy behavour.

Bye,
bearophile
Aug 24 2013
parent reply maarten van damme <maartenvd1994 gmail.com> writes:
I'm not sure if it really is my place to criticize design decisions of a
language written by people that devoted their career to computer science
while I have only read some books out of boredom :)



2013/8/25 bearophile <bearophileHUGS lycos.com>


 maarten van damme:


  But remove doesn't truly remove from the source range because the length

 of
 the source range stays the same. It's return value is a modified copy of
 the source range.

 Filter doesn't really work right away because that would also remove
 duplicate elements while I only want to remove at a given index. It also
 makes for clunkier looking code and is counterintinuitive to come up with
 (I want to remove an element therefore I have to filter every element that
 isn't equal to that element from the source range...)
 And while I haven't worked in c++, even that appears to have remove_copy
 which is really what I want.


 I agree that the design of std.algorithm.remove is bug prone. The compiler
 doesn't verify that you are using its result! I have also filed several
 bugs regarding the implementation of std.algorithm.remove.

 In the end your complaints are real. You can't expect Phobos to be
 perfect, but people work to fix it. Fixing such problems future D users
 will not find your problem. So I suggest you to think well about what your
 problem is, what you don't like on this, and then write a enhancement
 request for Phobos. Even if std.algorithm.remove can't be fixed, people can
 add a new function with a safer/more handy behavour.

 Bye,
 bearophile
Aug 24 2013
next sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
maarten van damme:


 I'm not sure if it really is my place to criticize design 
 decisions of a
 language written by people that devoted their career to 
 computer science
 while I have only read some books out of boredom :)


At the moment dpaste is not working for me.

Regarding your right to criticize, if someone gives you a car 
that has upside-down door handles, you are right to criticize, 
despite you have only 16 years old and no degree in mechanical 
engineering :-)

And regarding the knowledge from the books, there was a nice 
story written by Asimov (it's a bit long and it's a bit obsolete, 
but perhaps we'll never see a movie about it):
http://www.abelard.org/asimov.php

Bye,
bearophile
Aug 25 2013
prev sibling parent "John Colvin" <john.loughran.colvin gmail.com> writes:
On Sunday, 25 August 2013 at 03:58:26 UTC, maarten van damme 
wrote:

 I'm not sure if it really is my place to criticize design 
 decisions of a
 language written by people that devoted their career to 
 computer science
 while I have only read some books out of boredom :)


Sometimes an outside observer can make very useful observations. 
You might be able to see the wood while we're endlessly working 
away at the trees!
Aug 25 2013
prev sibling parent maarten van damme <maartenvd1994 gmail.com> writes:
Correction, I could really use remove to begin with, only have to dup the
range. Still convinced that the remove behaviour is counterintuitive but I
assume good reasons exist...


2013/8/25 maarten van damme <maartenvd1994 gmail.com>


 But remove doesn't truly remove from the source range because the length
 of the source range stays the same. It's return value is a modified copy of
 the source range.

 Filter doesn't really work right away because that would also remove
 duplicate elements while I only want to remove at a given index. It also
 makes for clunkier looking code and is counterintinuitive to come up with
 (I want to remove an element therefore I have to filter every element that
 isn't equal to that element from the source range...)
 And while I haven't worked in c++, even that appears to have remove_copy
 which is really what I want.

 I could use temporary variables and blow that neat line up in 3 lines
 which, while still readable, look redundant.

 I'm happy the d result is 3 times shorter then their haskell solution but
 not that the time spent was 5 minutes working and the rest of the time
 debugging. This as opposed to using racket where the time spent is 15
 minutes but after that everything works great (while my racket experience
 is limited to playing around with it for 2 weeks now and then).


 2013/8/25 Brad Anderson <eco gnuk.net>


 On Sunday, 25 August 2013 at 02:24:30 UTC, maarten van damme wrote:


 hello,

 I'm a hobyist-programmer and around where I live there's a group of
 haskell
 fanatics. They posted solutions to a recent programming challenge which I
 find to be a bit ugly. For fun I wanted to implement it in d and a rough
 version (not correct yet, this was written/hacked in 5 minutes after
 reading the exercise)

 My rough version is posted here : http://dpaste.dzfl.pl/4b5a6578

 if you look at the output, you'll see this particular line :
 "omkom -> komkom because of : kom momkom momkom -> momkomm"

 This is because of what remove from std.algorithm does. It not only
 returns
 a range with that element removed (as the name implies), it also modifies
 the original range.
 I assume this decision was made for efficiency purposes but that is one
 of

 'ref' in it's parameters so you know something's up. Is there any way I
 could've known this? (apart from reading the documentation on every
 single
 trivial function in the std library?)


 It was done that way intentionally because the purpose of remove is to
 remove from the source range.  If you don't want to affect the source range
 use filter.

 I suspect you could trace remove's lineage back to C++ STL's remove which
 works similarly (but is significantly clunkier and harder to use).
Aug 24 2013
prev sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
maarten van damme:


 Is there any way I could've known this?


For me it was "obvious" even before reading the documentation 
that a remove function is meant to be in-place. But of course you 
could have different ideas.



 (apart from reading the documentation on every single
 trivial function in the std library?)


Before using every function from the standard library you have to 
read its documentation, this is sure. You can not assume its 
behavour to be exactly the same you hope it to have. This is true 
for languages as Python too.

Regarding your very quickly written D code, I suggest to decrease 
the indents size, put a space around operators, put 
immutable/const to everything that doesn't need to change (unless 
there are problems doing so). I also suggest to use UFCS chains, 
because they are more readable for this kind of code. Instead of 
the readln()[0..$-1] you could chomp, strip or do not retain 
newlines. Instead of array(splitter) it's simpler to just split. 
Instead of chain.length==0 it's better to use empty. The foreach 
usually doesn't need a type, and probably your for loop could 
written as a better foreach. I also suggest to add contracts to 
your code. Instead of returning a "can't be solved", probably 
there are stronger typed solutions, like using an Algebraic or 
Nullable.

Bye,
bearophile
Aug 24 2013
parent maarten van damme <maartenvd1994 gmail.com> writes:
  (apart from reading the documentation on every single

 trivial function in the std library?)


 Before using every function from the standard library you have to read its
 documentation, this is sure. You can not assume its behavour to be exactly
 the same you hope it to have. This is true for languages as Python too.


While this is true, one can safely assume that a function called "writeln"
that takes a string will print that string and in the process not alter the
source. (Or at least, that's what I hope to be possible).



 Regarding your very quickly written D code, I suggest to decrease the
 indents size, put a space around operators, put immutable/const to
 everything that doesn't need to change (unless there are problems doing
 so).



I was timing the time it took me to go from exercise to working program. I
figured spamming immutable everywhere would only slow time down while not
increasing productivity (for small programs. If I were to write something
big, that is indeed a must)



 I also suggest to use UFCS chains, because they are more readable for this
 kind of code. Instead of the readln()[0..$-1] you could chomp, strip or do
 not retain newlines.



Hehe, I didn't knew about chomp, thanks for the tip.



 Instead of array(splitter) it's simpler to just split.


also a hidden gem. I always go for my goodies to std.algorithm, I didn't
expect there to be 'duplicates' in std.array as well.



 Instead of chain.length==0 it's better to use empty. The foreach usually
 doesn't need a type, and probably your for loop could written as a better
 foreach.


I like to spam types everywhere, it's something I've grown used to. I don't
like the concept of auto (although I do see why it could potentially
increase productivity).

I should've used a foreach loop, I can't remember why I didn't x)



 I also suggest to add contracts to your code. Instead of returning a
 "can't be solved", probably there are stronger typed solutions, like using
 an Algebraic or Nullable.


Yes, that was a bit ad-hoc decided. I was hesitating to use some kind of
bool for true-solved and false-unsolved and have the found parameter be by
ref. I came up with something that looks a bit neater now :)

http://dpaste.dzfl.pl/20034431
Aug 24 2013