Add default implementation of bit vector. #3
Conversation
This implementation is based on an array of long, and does not include 'rank' and 'select' yet.
|
I added some comments. Not sure if you get any email notifications from the inline/commit comments I made (they do not seem to count as comments in the issue view), so I am posting another one here. |
|
I received all comments in separate e-mail messages. |
The changes commented in the code review are already applied.
| * | ||
| * @return <code>true</code> if the element was successfully added. | ||
| */ | ||
| boolean add(boolean element); |
There was a problem hiding this comment.
Should this be "addBit"? (to match "getBit" and "setBit")
Also, could we call the value "value" like in setBit()? Or maybe best call it "bit" in all locations. The RankedBitVector uses "bit" already. setBit uses "value", add uses "element". I think "bit" is clearest.
|
I reviewed the new code. Mainly small comments about parameter naming based on our new style conventions (I guess my code also needs to be updated for this now ;-). Also some remarks on equals(). Rest looking good. Nice touch testing with the pseudo-random bits. Btw, if you want to continue with the ranked vector, maybe make a new branch so that this one can be merged soon; I guess it's ok to branch off from a feature branch that gets merged. |
|
|
||
| for (int i = 0; ret && (i < arraySize); i++) { | ||
| ret = ret | ||
| && (this.arrayOfBits[i] == otherBitVectorImpl.arrayOfBits[i]); |
There was a problem hiding this comment.
You can get rid of ret completely by replacing this line by
if (this.arrayOfBits[i] == otherBitVectorImpl.arrayOfBits[i]) return false;
(and similarly below). Then the last return ret just becomes return true. This improves efficiency since it stops scanning the vector as soon as a difference is found.
|
Looks good. I just have one last comment regarding efficiency of equals(). |
Add default implementation of bit vector.
This implementation is based on an array of long, and does not include
'rank' and 'select' yet.