search

contain-rs / bit-set

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

unnecessary use of pointers for remove and contains #5

Closed godel9 closed 8 years ago

godel9 commented 8 years ago

the remove and contains methods for bitset take an argument of type &usize, but there seems to be no reason for this instead of just taking a usize directly.

apasel422 commented 8 years ago

The reason is essentially consistency with the other set types (e.g. BTreeSet).

polazarus commented 8 years ago

Maybe some day there will be a trait for all Set. Until then, I don't think the consistency issue is very relevant.

Gankra commented 8 years ago

Arguably they should be taking an &Q where Q: Borrow<usize> to match the other collections.

solson commented 8 years ago

This was somewhat annoying in some cases where I was casting, so the calls looked like:

bitset.remove(&(value as usize));

I could buy @Gankro's argument, though.

Gankra commented 8 years ago

In VecMap we recently merged this change on the reasoning that any future collection trait can be used to paper over this difference. Happy to accept a PR.

solson commented 8 years ago

Fix in https://github.com/contain-rs/bit-set/issues/7.