contain-rs / bit-set

A Set of Bits
Apache License 2.0
63 stars 25 forks source link

Rename len() to count() to reflect its performance implication #22

Open upsuper opened 4 years ago

upsuper commented 4 years ago

In Rust, len() (as a noun) is commonly used to refer to something which can be figured out in constant time, usually reading a field or so, while the function here apparently is not just reading some field, but instead iterating through the internal vector and add up the number of set bits. For non-constant time operations, it should be preferred to use a verb name to indicate that it may not be cheap. I think the convention is to use count() for this scenario.

I also bumped the minor version as renaming a function is a breaking change. Let me know if you prefer keeping len() but having it deprecated so that we don't do breaking change for this.

pczarn commented 2 months ago

I would prefer to deprecate len just so users know about this change, and the transition is easier.

upsuper commented 2 months ago

Okay, I just updated it to mark len deprecated rather than replacing it completely.

pczarn commented 2 months ago

Thanks. I will merge within a few days and release with the next minor breaking version, "just in case", even if there is no need to bump minor (is that how semver works for version 0.x?)

pczarn commented 2 months ago

I will merge this after releasing 0.6.1 with recent bug fix