search

petgraph / fixedbitset

A simple bitset container for Rust
https://docs.rs/fixedbitset/
Apache License 2.0
124 stars 47 forks source link

Add is_clear method #79

Closed nicopap closed 2 years ago

nicopap commented 2 years ago

When using this library, the intent of checking if all elements in a bitset are disabled is better expressed with a specialized method. I doubt it's particularly more performant than bitset.count_ones(..) == 0, but when reading code using FixedBitSet, it's easier to understand what is going on with a bitset.is_clear() than a bitset.count_ones(..) == 0.

I also added docs so that it's less easy to get confused as to what "empty" means (I think the confusion was raised multiple times in the past)

jrraymond commented 2 years ago

Thanks. Can you please add some tests for this function?

nicopap commented 2 years ago

Thank for the review! Very important to keep thing names consistent in documentation. I indeed picked up "element" by just looking at the iterator methods. I'll fix that.

A question: the docs use both "enabled/disabled" and "set/unset" for value = 1/0. You seem to prefer "set/unset", and I'll follow this preference, but I prefer "enabled/disabled" since "set" can also mean a set of values.

nicopap commented 2 years ago

I'm very happy with the second pass on the docs. Although, in insight, I'm a bit skittish about both updating the doc on is_empty and len and adding is_clear.

jrraymond commented 2 years ago

"enabled/disabled" is ok. Using "bit" instead of element is more important.

Don't worry about updating the doc on is_empty and len.

nicopap commented 2 years ago

Thank you for the directions and the merge! Was the doc example code enough test?