On Wed, 22 Dec 2021 13:32:06 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> A cache that's manually invalidated and then validated when needed is a form >> of lazy evaluation. The main point, regardless of how you name it, is to >> ensure that every call that modifies the underlying BitSet invalidates the >> size. If it does, and it can be proven to do so, then that's sufficient. > > I checked, and I think that `set(int index, int... indices)` is fine, since > it doesn't modify `bitset` directly, and all of the methods it calls that do > modify it, correctly invalidate `size`. > > One more thing I spotted that needs to be checked, is the > `SelectedIndicesList(BitSet)` constructor. It saves a reference to the source > `BitSet` without making a copy. If a caller ever modified the source > `BitSet`, it would change the underlying `BitSet` seen by the > `SelectedIndicesList` class, and the size would therefore be wrong. Even if > the current code doesn't do this, it's somewhat fragile. I recommend adding a > comment to the constructor, and to the one place it's called, noting that the > source `BitSet` must not be modified after passing it to the constructor. > > Finally, there is a commented out `clearAndSelect` method that would break > the new contract of invalidating size whenever the bitmap is modified, if > that method were ever uncommented. Either it should be deleted as part of > this PR or it should be modified with a (commented out, of course) `size = > -1` in the right place(s). I have added some tests which calls public methods in `MultipleSelectionModelBase` and in turn calls methods from `SelectedIndicesList`. ------------- PR: https://git.openjdk.java.net/jfx/pull/673
