Remove IndexedInts.iterator()#4811
Conversation
|
@leventov would an |
|
CI failures are (partially?) from BitmapIndexedInts, where |
leventov
changed the title
leventov
changed the title
| } | ||
| }; | ||
|
|
||
| int size(); |
There was a problem hiding this comment.
It looks like the most common use for size() is as an isEmpty() check, but the size() method in some bitmap implementations is not straight forward. Would it make sense to get rid of size() and have a isEmpty() instead, or have an isEmpty() in addition to size()?
There was a problem hiding this comment.
oh wait, i was looking at ImmutableBitmap::size, so it might be outside the scope of this PR.
|
@gianm @drcrallen do you have more comments here? |
| @CalledFromHotLoop | ||
| int get(int index); | ||
|
|
||
| default void forEach(IntConsumer action) |
There was a problem hiding this comment.
Is this needed? This is used only in tests.
There was a problem hiding this comment.
Yes, I added this for nicer code in tests. I think it's ok
* Remove IndexedInts.iterator() * Retain IndexedInts.iterator(), but don't extend Iterable * Add BitmapValues * Fix tests
4 participants
To avoid accidental boxing for-each iteration like
for (Integer i : indexedInts) {...}, that did actually happen several times in production code.IndexableAdapter.getBitmapIndex()is renamed togetBitmapValues()and it's return type is changed toBitmapValues