You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Claude Warren (Jira)" <ji...@apache.org> on 2022/06/15 07:23:00 UTC
[jira] [Created] (COLLECTIONS-820) BloomFilter: renaming merge/mergeInPlace
Claude Warren created COLLECTIONS-820:
-----------------------------------------
Summary: BloomFilter: renaming merge/mergeInPlace
Key: COLLECTIONS-820
URL: https://issues.apache.org/jira/browse/COLLECTIONS-820
Project: Commons Collections
Issue Type: Improvement
Components: Collection
Affects Versions: 4.5
Reporter: Claude Warren
h3. !https://avatars.githubusercontent.com/u/886334?s=48&v=4|width=24,height=24! *[aherbert|https://github.com/aherbert]* [on 27 Feb|https://github.com/apache/commons-collections/pull/258#discussion_r813419143]
With the provision of the {{copy}} method the {{merge}} method seems redundant with {{{}mergeInPlace{}}}. It is equivalent to two lines of code:
{{BloomFilter filter2 = filter.copy(); }}
{{filter2.mergeInPlace(other);}}
I think {{merge}} should be the collections equivalent of {{{}add{}}}. This acts in place. Very few collections operations return new collections unless they are views of the current collection, e.g. List.subList. Note that the CountingBloomFilter has {{add}} which acts in place. There is no {{add}} returning a new CountingBloomFilter and then an {{{}addInPlace{}}}. So there is some inconsistency here.
IMO the more common operation is to merge a filter in place. So this should be the action of merge. Then the mergeInPlace can be dropped or replaced with {{copyAndMerge}} or {{{}mergeToCopy{}}}. You then also have the boolean result of merge to consider. Should the copy not be returned if the merge returns false? Since it is 2 lines of code I would not bother and drop this.
The BloomFilter then has {{merge}} and CountingBloomFilter has {{{}add{}}}, both act in place (the common operation). The user can create a copy if desired for a merge if that is useful to them.
Another comment on {{merge}} suggests changing it to support {{add}} and {{merge}} as both have their own use cases.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)