Jonathan M Davis wrote:

> So, as far as I can tell, the current situation is more efficient

There are some more arguments:

1) Different threads might want to `remove' the same key from the AA. I don't see a reason why only the first served thread should complete the operation without an error.

2) Because there is only one operation for insertion of a key into an AA, only one operation should be used for removing that key.

Note: This argument vanishes if insertions are divided in
  a:declaration of a key as valid in that AA,
  b:first assignment to the value of a key,
  c:reassignment to the value of a key

3) Reverse operations should not implement more functionality than the reversed operation. Because `remove' is the reverse operation for inserting a key and that operation does not cause any error, neither should errors be given from `remove'.

Note: Reassigning to the value of a key might very well treated as an error.

-manfred

For most languages (such as C# and maybe Java), the Remove method on collections returns a boolean indicating whether it was removed. So you could write:

enforce(MyAA.remove("lenght"))
or
bool Result = MyAA.remove("lenght");
assert(Result);

I'm not sure why it doesn't in D, but I thought I remembered seeing a pull request or change that added it. Maybe I'm imagining things since I can't find it now.

On 07/01/2012 4:11 PM, RenatoL wrote:
> Very quick question
>
> import std.stdio;
> void main()
> {
> 	auto arr1 = ["uno":1, "due":2, "tre":3];
> 	arr1.remove("xxx");
> }
>
> also in this case the compiler does not say anything and the
> program goes out silently ... why? Would not it be better if an
> exception was raised? After all if i write:
>
> writeln(arr1["xxx"]);
>
> runtime expresses its disappointment...

On 1/7/2012 8:54 PM, bearophile wrote:
>> Yes, Jonathan, you're right.
>> the question arose precisely from a typo... i had to remove an
>> item with key "length"... i wrote "lengt" and the item never went
>> away... i knew that "lengt" was not in my key list... This kind of
>> mistake is quite tricky, may be using and IDE could help.
> 
> This an example that shows that silent failures are sources of bugs... So this time I don't agree with Jonathan Davis.

a.remove(x) doesn't fail though. It successfully satisfies its own postconditions by placing a in a state whereby it no longer contains x. The real source of bugs in this example is using hardcoded strings instead of named constants. :)

Besides, one frequently doesn't know if (x in a) or not ahead of time. Think of evicting some computed intermediate results from a cache when you detect that the underlying resource has changed, for instance. In fact, it's really rare for an item to be removed from a long-lived collection anywhere near the point that it was added. Insisting on existence before removal clutters up the code for an extremely common use case.

FWIW, Java's Map and .NET's IDictionary also handle this situation silently.

Regards,
~Stephen

On Sunday, January 08, 2012 01:39:24 Kapps wrote:
> For most languages (such as C# and maybe Java), the Remove method on collections returns a boolean indicating whether it was removed. So you could write:
> 
> enforce(MyAA.remove("lenght"))
> or
> bool Result = MyAA.remove("lenght");
> assert(Result);
> 
> I'm not sure why it doesn't in D, but I thought I remembered seeing a pull request or change that added it. Maybe I'm imagining things since I can't find it now.

There _is_ extra cost in returning bool rather than void, but I'm not at all sure that it's unreasonable to incur that cost. std.container.RedBlackTree's remove function returns how many elements were removed. Other functions in Phobos do similar in similar situations.

It probably merits an enhancement request.

- Jonathan M Davis

Ah, found the bug / pull request.

https://github.com/D-Programming-Language/dmd/pull/597
http://d.puremagic.com/issues/show_bug.cgi?id=4523

On 08/01/2012 1:39 AM, Kapps wrote:
> For most languages (such as C# and maybe Java), the Remove method on
> collections returns a boolean indicating whether it was removed. So you
> could write:
>
> enforce(MyAA.remove("lenght"))
> or
> bool Result = MyAA.remove("lenght");
> assert(Result);
>
> I'm not sure why it doesn't in D, but I thought I remembered seeing a
> pull request or change that added it. Maybe I'm imagining things since I
> can't find it now.
>
> On 07/01/2012 4:11 PM, RenatoL wrote:
>> Very quick question
>>
>> import std.stdio;
>> void main()
>> {
>> auto arr1 = ["uno":1, "due":2, "tre":3];
>> arr1.remove("xxx");
>> }
>>
>> also in this case the compiler does not say anything and the
>> program goes out silently ... why? Would not it be better if an
>> exception was raised? After all if i write:
>>
>> writeln(arr1["xxx"]);
>>
>> runtime expresses its disappointment...
>

On Sunday, January 08, 2012 03:24:22 Kapps wrote:
> Ah, found the bug / pull request.
> 
> https://github.com/D-Programming-Language/dmd/pull/597 http://d.puremagic.com/issues/show_bug.cgi?id=4523

Ah, TDPL says that it returns bool. Well, then it definitely needs to be changed, and it's good to see that that there's a pull request for it. So, it should be fixed in the next release. In fact, the number of TDPL-related bugs fixed for the next release should be quite high, which is great.

- Jonathan M Davis

On 08.01.2012 10:43, Jonathan M Davis wrote:
> On Sunday, January 08, 2012 03:24:22 Kapps wrote:
>> Ah, found the bug / pull request.
>>
>> https://github.com/D-Programming-Language/dmd/pull/597
>> http://d.puremagic.com/issues/show_bug.cgi?id=4523
>
> Ah, TDPL says that it returns bool. Well, then it definitely needs to be
> changed, and it's good to see that that there's a pull request for it. So, it
> should be fixed in the next release. In fact, the number of TDPL-related bugs
> fixed for the next release should be quite high, which is great.
>
> - Jonathan M Davis

Wouldn't it make sense to return a pointer to the item being removed/null? This way you can continue to work with the item, and it would be consistent with `in`.

if (auto item = aa.remove(key)) {
  // some additional work on item
} else { // not found
  // throw or something
}

simendsjo wrote:

> Wouldn't it make sense to return a pointer to the item being removed/null?

According to the docs this is the intended behavior.

-manfred

On 08.01.2012 11:09, Manfred Nowak wrote:
> simendsjo wrote:
>
>> Wouldn't it make sense to return a pointer to the item being
>> removed/null?
>
> According to the docs this is the intended behavior.
>
> -manfred

The only mention I can see of remove is at the top, and it doesn't state return value: http://dlang.org/hash-map.html

The pull changes the return value to bool: https://github.com/9rnsr/dmd/commit/f3cc75445775927e2036054811afb686cf4f1624

On Sunday, January 08, 2012 11:02:41 simendsjo wrote:
> On 08.01.2012 10:43, Jonathan M Davis wrote:
> > On Sunday, January 08, 2012 03:24:22 Kapps wrote:
> >> Ah, found the bug / pull request.
> >> 
> >> https://github.com/D-Programming-Language/dmd/pull/597 http://d.puremagic.com/issues/show_bug.cgi?id=4523
> > 
> > Ah, TDPL says that it returns bool. Well, then it definitely needs to be changed, and it's good to see that that there's a pull request for it. So, it should be fixed in the next release. In fact, the number of TDPL-related bugs fixed for the next release should be quite high, which is great.
> > 
> > - Jonathan M Davis
> 
> Wouldn't it make sense to return a pointer to the item being removed/null? This way you can continue to work with the item, and it would be consistent with `in`.
> 
> if (auto item = aa.remove(key)) {
>    // some additional work on item
> } else { // not found
>    // throw or something
> }

Since the element has been removed from the container, what would the pointer point to? The element doesn't exist anymore.

- Jonathan m Davis