sdleffler / qp-trie-rs

An idiomatic and fast QP-trie implementation in pure Rust.
Mozilla Public License 2.0
95 stars 24 forks source link

Match `std::collections` API #11

Open tapeinosyne opened 6 years ago

tapeinosyne commented 6 years ago

For collections, it is generally desirable to match the standard library API where feasible. By and large, qp_trie already offers the methods one would expect from a trie, but there remain a few edges where the interface could be smoothed out to better fit the expectations created by std.

The missing methods that can be immediately implemented with no internal changes are:

Some that would require some work or discussion, but are clearly useful:

At a glance, with the QP-tries being recursive, methods related to preallocation would require some significant restructuring to be effective, and the returns might be suspect:

(I jotted down a patch for the methods in the first section, and will follow up on the rest later on.)

sdleffler commented 6 years ago

I'll note that BTreeMap doesn't have reserve/with_capacity, so there is precedent for leaving it out.

sdleffler commented 6 years ago

append wouldn't be hard to implement in the same vein as BTreeMap, since a trie is inherently sorted. The same approach could be taken - create a "merged" iterator from the two tries and then construct a third trie from it.

sdleffler commented 6 years ago

drain is an interesting one for a trie. The drain method on Vec allows for a range to be drained - the drain method on HashMap drains the entire HashMap; BTreeMap, on a cursory investigation, doesn't appear to have a drain method. It would be easy to implement drain like HashMap - simply mem::replace the trie itself with a new one, and then into_iter the old. However, as a trie is an ordered mapping, it might make sense (and be fairly simple to) implement a drain for all elements under a given prefix (implemented as a remove_prefix and then .into_iter.) It might also make sense to implement a drain for all elements in a range... but that's a bit more difficult.

erikbrinkman commented 1 year ago

I think the reason drain doesn't exist on a BTreeMap is that the only reason to use drain vs move in a hasmap is that it preserves the allocated memory. That's not possible with a BTreeMap, so drain is no better than std::mem::take.