[
https://issues.apache.org/jira/browse/LUCENE-3449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13112766#comment-13112766
]
Michael McCandless commented on LUCENE-3449:
--------------------------------------------
I don't like that this adds an additional if inside next/prevSetBit
that for some cases is not needed (eg BlockJoinQuery's usage).
I think performance trumps convenience here for these low level APIs.
Why not use a simple while loop?
{noformat}
int idx = 0;
while (idx < length) {
idx = bits.nextSetBit(idx);
if (idx == -1) {
break;
}
// do something with idx
idx++;
}
{noformat}
Nomatter what, this loop needs 2 ifs per cycle; I just think that 2nd
if belongs in the loop itself and not inside next/prevSetBit, since
not all use cases require it.
> Fix FixedBitSet.nextSetBit/prevSetBit to support the common usage pattern in
> every programming book
> ---------------------------------------------------------------------------------------------------
>
> Key: LUCENE-3449
> URL: https://issues.apache.org/jira/browse/LUCENE-3449
> Project: Lucene - Java
> Issue Type: Improvement
> Components: core/other
> Affects Versions: 3.4, 4.0
> Reporter: Uwe Schindler
> Priority: Minor
> Attachments: LUCENE-3449.patch
>
>
> The usage pattern for nextSetBit/prevSetBit is the following:
> {code:java}
> for(int i=bs.nextSetBit(0); i>=0; i=bs.nextSetBit(i+1)) {
> // operate on index i here
> }
> {code}
> The problem is that the i+1 at the end can be bs.length(), but the code in
> nextSetBit does not allow this (same applies to prevSetBit(0)). The above
> usage pattern is in every programming book, so it should really be supported.
> The check has to be done in all cases (with the current impl in the calling
> code).
> If the check is done inside xxxSetBit() it can also be optimized to be only
> called seldom and not all the time, like in the ugly looking replacement,
> thats currently needed:
> {code:java}
> for(int i=bs.nextSetBit(0); i>=0; i=(i<bs.length()-1) ? bs.nextSetBit(i+1) :
> -1) {
> // operate on index i here
> }
> {code}
> We should change this and allow out-of bounds indexes for those two methods
> (they already do some checks in that direction). Enforcing this with an
> assert is unuseable on the client side.
> The test code for FixedBitSet also uses this, horrible. Please support the
> common usage pattern for BitSets.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
