CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
12.75k stars 3.46k forks source link

Make collections iterable #11592

Open angrycat9000 opened 11 months ago

angrycat9000 commented 11 months ago

Right now to iterate over a collection I have do a for loop with the index and then call get to get the object.

for(let i = 0; i < viewer.imageryLayers.length; i++ ) {
  const layer = viewer.imageryLayers.get(i);
  // do something with layer
}

It would be nicer if the collections provided iterators so I could do:

for(const layer of viewer.imageryLayers) {
   // do something with layer
}
javagl commented 9 months ago

I like the concepts of Iterators and Generators (also "Streams" in other languages), and think that this could be a low-hanging fruit to increase usability, readability, and maintainability. Particularly in cases where people felt the urge to pull out the collection and its length...

const theLayers = viewer.imageryLayers;
const theLength = thelayers.length;
for(let i = 0; i < theLength; i++ ) {
  const layer = theLayers.get(i);
  // do something with layer
}

because of ~"performance".

(An aside: One could be tempted to quote Knuth here and talk about "premature optimization". And ... if someone claims that pulling the length into a constant increases the performance, then I'd like to see the benchmark. But... due to quirks of the language, this may actually make a difference in some cases - at least theoretically: Nobody knows whether something like collection.length only returns a value of a propery, or whether it calls a 'getter' that internally iterates the whole collection or whatnot...)

These point is: There are many possible considerations for "how to write 'good' code" (and we aren't even talking about i++ vs. ++i yet). All these questions could become obsolete, when the answer is: "'Idiomatic code' is 'good code' - just use a standard for...of loop".

Still, addressing this in all depth does raise further questions.

One is whether the coding guide at https://github.com/CesiumGS/cesium/blob/main/Documentation/Contributors/CodingGuide/README.md should be updated accordingly (it uses the const length = ... pattern a lot...).

Another one is whether we should

  1. try to find some sensible "benchmark" run that is representative for "CesiumJS as a whole"...
  2. change ALL iterations in the whole CesiumJS codebase from the i < length pattern to the for...of pattern
  3. do the same benchmark run again, and see whether there is a difference

A draft PR with a few, first iterators for the ...Collection classes is at https://github.com/CesiumGS/cesium/pull/11697