65

Safe code for new Set() may look like:

let items = [];
for (let item of set)
  if (isBad(item))
    items.push(item);
for (let item of items)
  set.delete(item)

Can I simplify code to:

for (let item of set)
  if (isBad(item))
    set.delete(item);

Safe code for new Map() may look like:

let keys = [];
for (let [key, val] of map)
  if (isBadKey(key) || isBadValue(val))
    keys.push(key);
for (let key of keys)
  map.delete(key)

Can I simplify code to:

for (let [key, val] of map)
  if (isBadKey(key) || isBadValue(val))
    map.delete(key)
gavenkoa
  • 45,285
  • 19
  • 251
  • 303

2 Answers2

87

Yes, you can simplify to that, it's totally safe.

  • Sets and Maps are always iterated in insertion order
  • Deleting an item does not affect the position of any iterator - you can visualise the shape of the collection not being changed, just being emptied.
  • So: elements that are deleted and have not yet been iterated won't be iterated
  • Elements that have already been iterated and are deleted (like in your case) won't affect anything but other iterations/lookups.
  • Elements that are added (and are not already part of the collection) during the iteration will always be iterated

From that last point follows that the only dangerous thing to do would be something like

const s = new Set([1]);
for (let x of s) {
    s.delete(x);
    s.add(1);
}

but not because of undefined behaviour or memory accumulation, but because of the infinite loop.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • 4
    Can you please add a link to reference documentation that corroborates your statement? – jtbandes Jul 21 '19 at 05:48
  • @jtbandes I don't know any documentation that states exactly this (you might want to try MDN however), the conclusions in my answer directly follow the [observable semantics of collections in the spec](http://ecma-international.org/ecma-262/#sec-keyed-collections) as lists of entries. – Bergi Jul 21 '19 at 13:32
  • 1
    There's a note under [Map#forEach](http://ecma-international.org/ecma-262/#sec-map.prototype.foreach) spec saying similar things. – wOxxOm Oct 01 '19 at 07:22
  • 5
    You should say "Yes it's safe" instead of just "Yes" because the title starts by "Is it dangerous...". – Cinn Apr 24 '21 at 17:03
8

I would say yes, it's safe. When you iterate over the Set/Map using for ... of under the hood the loop is going through @@iterator. And Iterator operates with .next() only: so no indices and no matter what is before the current position. Only one next element is important.

So until you remove elements "in front of" the current iterator position - it's safe to do it.

Kiril
  • 2,935
  • 1
  • 33
  • 41
  • What if `Set()` implemented as binary tree? Removing node may cause tree re-balancing. Does it damage iterator operation? – gavenkoa Mar 11 '16 at 12:53
  • 1
    It's not. According to spec - inside [Set](https://tc39.github.io/ecma262/#sec-set-iterable) is more like a [List](https://tc39.github.io/ecma262/#sec-list-and-record-specification-type): `Set set's [[SetData]] internal slot to a new empty List.` – Kiril Mar 11 '16 at 12:57
  • I see description of `Set` in term of List. So is it naive assumption that `Set` perform `delete`/`add`/`has` in `O(log2(size))` time but actually in `O(size)`? – gavenkoa Mar 11 '16 at 13:04
  • 2
    The spec only defines required behavior, not how it should be implemented. Implementations can - for example - achieve the defined behavior by implementing it as a hash set with insertion-ordered linked set entries, thus achieving amortized O(1) lookups while maintaining a predictable order. – the8472 Mar 11 '16 at 13:22
  • 1
    @gavenkoa about deleting - it's [O(size)](https://tc39.github.io/ecma262/#sec-set.prototype.delete). – Kiril Mar 11 '16 at 14:01
  • @the8472 I'd assume that all browsers should do their implementations according to spec. That what it was created for :) – Kiril Mar 11 '16 at 14:01
  • 3
    @Bergi, according to spec - [Yes](https://tc39.github.io/ecma262/#sec-set.prototype.delete): `Repeat for each e that is an element of entries,` - so one loop over all entries. But, of course, only if it's implemented according to spec. – Kiril Mar 11 '16 at 16:44
  • @Kiril: That's how the spec says it should behave, but the spec says it's not how it should be implemented. – Bergi Mar 12 '16 at 10:53
  • The Map and Set spec notes at the beginning: “Map object must be implemented using either hash tables or other mechanisms that, on average, provide access times that are **sublinear** on the number of elements in the collection. The data structures used in this Map objects specification is only intended to describe the required observable semantics of Map objects. **It is not intended to be a viable implementation model.**” – Thai Apr 25 '20 at 19:55