bbc / peaks.js

JavaScript UI component for interacting with audio waveforms
https://waveform.prototyping.bbc.co.uk
GNU Lesser General Public License v3.0
3.16k stars 277 forks source link

[Question] peaks.points.removeAll() does not reset internal "_pointIdCounter" - why? #428

Closed rowild closed 2 years ago

rowild commented 2 years ago

At a certain moment in the app I need to remove all points:

console.log('BEFORE this.peaks.points =', this.peaks.points);
this.peaks.points.removeAll()
console.log('AFTER this.peaks.points =', this.peaks.points);

The before and after console logs tell me that the _points array is indeed set to 0, but _pointIdCounter is not, as can be seen in the screenshot. I am not sure if this is important at all, since it does not cause any troubles (at least none that I know of).

Assuming that _pointIdCounter simply counts the number of points added to peaks, I wonder, if this shouldn't be reset, too, once removeAll has been called? Or are there any other involvements I am not aware of?

Thank you!

Bildschirmfoto 2021-11-22 um 22 20 41 t

chrisn commented 2 years ago

The idea behind this was that each point should have a unique id, and so a counter was a simple solution. We expected applications to treat these as opaque values, so didn't see a need to reset the counter. Also, resetting the counter means reusing values which could lead to the same id referring to an existing point as well as a removed point. I don't know whether that's important in practice, but those are the reasons we don't reset the counter. Any application wants to control the ids can set its own values when adding points.