search

tweedegolf / sequential-storage

A crate for storing data in flash memory with minimal need for erasing pages
Apache License 2.0
87 stars 8 forks source link

Default `Key` implementation for `&str` #50

Closed ryan-summers closed 3 months ago

ryan-summers commented 3 months ago

It seems like allowing a &str to have an automatic implementation for map::Key would be useful. We can encode the &str as bytes quite easily with a well-defined format.

Would this be something that would be desired? The implementation is quite small and I can make a PR for it. Curious if this was intentionally omitted for some reason.

ryan-summers commented 3 months ago

Ah hmm, I think this is a problem because &str doesn't implement Clone, which the trait currently requires. Would it make sense to relax this bound and instead borrow the key throughout the stack instead?

ryan-summers commented 3 months ago

Ah, looks like https://github.com/tweedegolf/sequential-storage/issues/39 is pointed at this. Closing, as there's additional complexity because of the key cache.

diondokter commented 3 months ago

I agree it'd be useful, however they keys needs to be able to be owned for the KeyPointerCache. So I don't think this can be done sadly.

Maybe with some trait-fu it could be done that if the cache doesn't need to own the cache you could use &str... But then if you care about performance you want to use that cache anyways...

ryan-summers commented 3 months ago

It's easy enough to wrap this all in a heapless::String type to get around the ownership problems, but I don't think that should live in this crate.

diondokter commented 3 months ago

I've already added an arrayvec feature that implements the key trait for ArrayVec and ArrayString, so that scenario is already covered. The Value trait is also implement for &[u8], so that even works without extra copies to e.g. an ArrayVec