[Issue 17526] Add a set method for AA

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17526

Iain Buclaw  changed:

   What|Removed |Added

   Priority|P1  |P4

--

[Issue 17526] Add a set method for AA

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17526

Stanislav Blinov  changed:

   What|Removed |Added

 CC||stanislav.bli...@gmail.com

--- Comment #8 from Stanislav Blinov  ---
This issue probably needs to be repurposed: since
https://github.com/dlang/druntime/commit/0c92d13c7f8540bd91c3cce251d97ff39b84a486
(present in 2.082) there is now a require() function that should be able to
achieve the desired behavior, but it's implementation at the moment does not
account for const value type.

--

[Issue 17526] Add a set method for AA

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17526

Martin Nowak  changed:

   What|Removed |Added

 CC||c...@dawg.eu

--- Comment #7 from Martin Nowak  ---
That's definitely on the list, I proposed getOrSet as name.
http://forum.dlang.org/search?q=getOrSet
https://github.com/dlang/druntime/pull/1282/files#diff-7f36fe9957b2e56c0c468548c4e1b0aaR133

It could be added to today's hashtable using a small variation of _aaGetY
internally (taking a delegate for the initialization of the value).

--

[Issue 17526] Add a set method for AA

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17526

Seb  changed:

   What|Removed |Added

 CC||greensunn...@gmail.com

--

[Issue 17526] Add a set method for AA

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17526

--- Comment #6 from Bolpat  ---
@Vladimir Exactly. It's a performance and possibility issue. We can discuss the
name after everything else is done.
I propose a change to the signature + semantics:

  V* set(K, V)(ref const(V)[const(K)] aa, auto ref const(K) key, lazy const(V)
value)
  {
if (auto valPtr = key in aa) return null;
// cast away const as initialization is okay:
(cast() aa[key]) = value();
return key in aa;
  }

So it returns null if it was not set and a pointer to the object in the AA to
be able to modify it after setting without additional lookups. I like addOrGet
very much. Both methods have their range of applicability. With this one, you
can do

  if (auto valPtr = aa.set(key, val()))
  {
valPtr.property = something();
manage(*valPtr);
  }

The main difference between the two is what you intend to do:
  - Yours assures the key has a value afterwards. It doesn't really care if the
key is there already. One cannot even know using it.
  - Mine expects the key not to be there beforehand. It is specifically
designed to add this key-value pair.

--

[Issue 17526] Add a set method for AA

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17526

--- Comment #5 from Vladimir Panteleev  ---
BTW, the performance use case of the proposed set method (which I called
getOrAdd in my hashmap implementation):

struct S { int i, j, k, l, m; /* etc. */ }
S[int] aa;

// The goal is to increment aa[x].k
// If aa[x] doesn't exist, initialize it with S.init
// Currently, you have to do this:

if (auto p = x in aa)
(*p).k++;
else
{
S s;
s.k = 1;
aa[x] = s;  // Wasteful - double lookup
}

--

[Issue 17526] Add a set method for AA

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17526

--- Comment #4 from Vladimir Panteleev  ---
(In reply to hsteoh from comment #3)
> What about a const overload to opIndexAssign that lets you assign to a new
> key, but aborts if the key already exists?

I'm not sure; I think the semantics would be a bit surprising. E.g. generally
if is(typeof(aa[key]=value)) is true, you'd expect it to work at runtime as
well (for POD types, at least).

> I'm a little hesitant about
> adding a whole new method to set a key in an AA.

I think it would be useful, but I agree it's a bigger addition that might use
more discussion. Not enough for a DIP, but it could be discussed in the pull
request if one were to make one.

I know Martin wants to give a templated AA implementation another go soon:
https://wiki.dlang.org/Vision/2017H2_draft

--

[Issue 17526] Add a set method for AA

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17526

--- Comment #3 from hst...@quickfur.ath.cx ---
@Vladimir: you have a good point, it should still be possible to append stuff
to the AA.

What about a const overload to opIndexAssign that lets you assign to a new key,
but aborts if the key already exists?  I'm a little hesitant about adding a
whole new method to set a key in an AA.  I'm not sure what to do about the
abort... throw an AssertError? RangeError?

--

[Issue 17526] Add a set method for AA

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17526

--- Comment #2 from Vladimir Panteleev  ---
(In reply to hsteoh from comment #1)
> If I have an array A of type const(int)[], I'd expect that I should not be
> able to assign new values to A[0], otherwise it breaks the const guarantee.

If you have an empty array of const(int)[], you can't assign to its elements,
but you can still append to it. In the same way, with an AA of type
const(int)[string], there's no reason why you shouldn't be able to grow the AA.

Such a function would be good to have for performance reasons as well (as
mentioned in passing by OP).

--

[Issue 17526] Add a set method for AA

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17526

hst...@quickfur.ath.cx changed:

   What|Removed |Added

 CC||hst...@quickfur.ath.cx

--- Comment #1 from hst...@quickfur.ath.cx ---
I'm not sure about this.  Isn't the fact that you declared the AA's value type
to be const a declaration that it cannot change once you initialize it?

If I have an array A of type const(int)[], I'd expect that I should not be able
to assign new values to A[0], otherwise it breaks the const guarantee.

Similarly, if I have an AA of type const(int)[string], I'd expect that I cannot
assign to AA["a"] once the AA is initialized, otherwise I'm violating the const
guarantee.

--