std.algorithm.remove and principle of least astonishment

[Andrei Alexandrescu]

On 10/16/2010 01:29 PM, klickverbot wrote:
> Hello all,
>
> I decided to have a go at solving some easy programming puzzles with
> D2/Phobos to see how Phobos, especially ranges and std.algorithm, work
> out in simple real-world use cases (the puzzle in question is from
> hacker.org, by the way).
>
> The following code is a direct translation of a simple problem
> description to D (it is horrible from performance point of view, but
> that's certainly no issue here).
>
> ---
> import std.algorithm;
> import std.conv;
> import std.stdio;
>
> // The original input string is longer, but irrelevant to this post.
> enum INPUT = "93752xxx746x27x1754xx90x93xxxxx238x44x75xx087509";
>
> void main() {
> uint sum;
>
> auto tmp = INPUT.dup;
> size_t i;
> while ( i < tmp.length ) {
> char c = tmp[ i ];
> if ( c == 'x' ) {
> tmp = remove( tmp, i );
> i -= 2;
> } else {
> sum += to!uint( [ c ] );
> ++i;
> }
> }
>
> writeln( sum );
> }
> ---
>
> Quite contrary to what you would expect, the call to »remove« fails to
> compile with the following error messages: »std/algorithm.d(4287):
> Error: front(src) is not an lvalue« and »std/algorithm.d(4287): Error:
> front(tgt) is not an lvalue«.
>
> I am intentionally posting this to this NG and not to d.…D.learn, since
> this is a quite gross violation of the principle of least surprise in my
> eyes.
>
> If this isn't a bug, a better error message via a template constraint or
> a static assert would be something worth looking at in my opinion, since
> one would probably expect this to compile and not to fail within Phobos
> code.
>
> David

Thanks for the input. This is not a bug, it's what I believe to be a 
very intentional feature: strings are not ordinary arrays because 
characters have variable length. As such, assigning to "the first 
character in a string" is not allowed because the assignment might mess 
up the next character.

It's a good test bed. Simply replacing this:

    auto tmp = INPUT.dup;

with this:

    auto tmp = cast(ubyte[]) INPUT.dup;

makes the program work and print 322 (you also must include std.conv).

How do you all believe we could improve this example?

1. remove() could be specialized for char[] and wchar[] because it can 
be made to work with some effort and is a worthwhile algorithms for 
strings.

2. to!(ubyte[]) should work for char[] by making a copy and casting it 
to ubyte[]. So this should have worked:

    auto tmp = to!(ubyte[])(INPUT);

to! is better than cast because it always does the right thing and never 
undermines type safety.

Whadday'all think?


Andrei