add iter_from to CountedMap
#10524
add iter_from to CountedMap
#10524
Conversation
kianenigma
added
A0-please_review
| /// | ||
| /// If you alter the map while doing this, you'll get undefined results. | ||
| pub fn iter_from( | ||
| starting_raw_key: Vec<u8>, |
There was a problem hiding this comment.
I just noticed this, but we really can just use &[u8] here instead of mandating a Vec here to allow for a more flexible API.
There was a problem hiding this comment.
If you want to have a flexible api, you should use impl Into<Vec<u8>>
There was a problem hiding this comment.
What's the benefit compared to &[]?
There was a problem hiding this comment.
all in all though @KiChjang I think if we change the API here, we should do so as well for many other exposed functions as well in the storage API, so I'd rather do it in a separate PR. For now, I prefer staying consistent with the rest of the API.
There was a problem hiding this comment.
these are zero cost abstractions right? in which case, yeah let's follow up on this
| /// Enumerate all elements in the counted map after a specified `starting_raw_key` in no | ||
| /// particular order. |
There was a problem hiding this comment.
is this true?
are they not ordered by key? else if they are in no particular order, why would we have a starting key as useful at all?
There was a problem hiding this comment.
They are probably ordered by key, however I don't think that we want to "force" ourselves to this "promise", so keeping the api more open to future implementations.
There was a problem hiding this comment.
without this constraint, the api seems useless. I get you want flexibility, but.... could just be confusing for someone who wants to use this.
There was a problem hiding this comment.
Why? You probably mainly want to iterate.
As the keys are hashed, you also can not be sure that the ordering based on the "decoded" key would be different to the ordering because of hashing the keys.
| /// Enumerate all elements in the counted map after a specified `starting_raw_key` in no | ||
| /// particular order. |
There was a problem hiding this comment.
They are probably ordered by key, however I don't think that we want to "force" ourselves to this "promise", so keeping the api more open to future implementations.
No description provided.