[
https://issues.apache.org/jira/browse/COLLECTIONS-826?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claude Warren resolved COLLECTIONS-826.
---------------------------------------
Resolution: Fixed
fixed by https://github.com/apache/commons-collections/pull/314
> BloomFilter: move CountingLongPredicate
> ---------------------------------------
>
> Key: COLLECTIONS-826
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-826
> Project: Commons Collections
> Issue Type: Improvement
> Components: Collection
> Affects Versions: 4.5
> Reporter: Claude Warren
> Assignee: Claude Warren
> Priority: Minor
> Labels: bloom-filter
>
>
> {noformat}
> class CountingLongPredicate implements LongPredicate {
> int idx = 0;
> final long[] ary;
> final LongBiPredicate func;
> CountingLongPredicate(long[] ary, LongBiPredicate func) {
> this.ary = ary;
> this.func = func;
> }
> @Override
> public boolean test(long other) {
> return func.test(idx == ary.length ? 0 : ary[idx++], other);
> }
> boolean forEachRemaining() {
> while (idx != ary.length && func.test(ary[idx], 0)) {
> idx++;
> }
> return idx == ary.length;
> }
> }
> {noformat}
>
> Since this is in the interface it is public. It should be an implementation
> detail. Hiding it ensures it is a one time use only and prevents strangeness
> when calling forEachRemaining (see below).
> idx will be initialsed as zero anyway so this can be removed: int idx = 0;
> @aherbert aherbert on 27 Feb
> If func.test returns false the loop will terminate before idx == ary.length.
> So we return false. However this allows forEachRemaining to be called again.
> This will use the same position in the array again. If public (currently it
> is not) then the method could be invoked multiple times and the behaviour is
> not defined (especially since you have added no comment on this).
> However the class is public but the forEachRemaining method is package
> private. I suggest moving this out of the interface so the class is package
> private.
> Note: I tested a spliterator from the List returned from Arrays.asList. The
> forEachRemaining can be invoked one time only. This behaviour can be obtained
> by caching idx and setting it to the array length:
> {noformat}
> boolean forEachRemaining() {
> int i = idx;
> final long[] a = ary;
> final int limit = a.length;
> if (i != limit) {
> // Prevent subsequent calls
> idx = limit;
> while (i != limit && func.test(a[i], 0)) {
> i++;
> }
> }
> return i == limit;
> }
> {noformat}
> However the code above as it stands could fail-fast and return false. Then be
> invoked again, do nothing with the func but will return true. The idx could
> be set above limit but this will only work if limit is not Integer.MAX_VALUE
> (which is unlikely). This can be handled by using compared unsigned:
> {noformat}
> boolean forEachRemaining() {
> int i = idx;
> final long[] a = ary;
> final int limit = a.length;
> if (Integer.compareUnsigned(i, limit) < 0) {
> while (i != limit && func.test(a[i], 0)) {
> i++;
> }
> // Set the result for repeat calls
> idx = i == limit ? i : -1;
> }
> return i == limit;
> }
> {noformat}
> This is over engineering a simple helper class. However a tiny optimisation
> to use local references would be of benefit. So make the class package
> private, add some javadoc and do:
> {noformat}
> /**
> * Call the long-long consuming bi-predicate for each remaining unpaired long
> in
> * the input array. This method should be invoked after the predicate has been
> * passed to {@link BitMapProducer#forEachBitMap(LongPredicate)} to consume
> any
> * unpaired bitmaps. The second argument to the bi-predicate will be zero.
> *
> * @return true if all calls the predicate were successful
> */
> boolean forEachRemaining() {
> int i = idx;
> final long[] a = ary;
> final int limit = a.length;
> while (i != limit && func.test(a[i], 0)) {
> i++;
> }
> return i == limit;
> }
> {noformat}
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
