Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

map/try_map for BoundedBTreeMap #11866

Closed
benluelo opened this issue Jul 19, 2022 · 6 comments · Fixed by #12581
Closed

map/try_map for BoundedBTreeMap #11866

benluelo opened this issue Jul 19, 2022 · 6 comments · Fixed by #12581
Labels
J0-enhancement An additional feature request. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.

Comments

@benluelo
Copy link
Contributor

benluelo commented Jul 19, 2022

It would be useful if there was a way to iterate over an &mut BoundedBTreeMap and mutate each key in place. This wouldn't affect the length of the map at all, so it would be safe. Perhaps a try_map or something similar would be good as well, to allow for short circuiting.

I would be happy to take this on if it's deemed useful.

(BoundedBTreeMap link for the lazy:

pub struct BoundedBTreeMap<K, V, S>(BTreeMap<K, V>, PhantomData<S>);
)

EDIT:

After some experimentation, I think this is a better API:

pub fn map<T>(self, mut f: impl FnMut((&K, V)) -> T) -> BoundedBTreeMap<K, T, S> {
    self.0
        .into_iter()
        .map(|(k, v)| {
            let t = f((&k, v));
            (k, t)
        })
        .try_collect()
        // SAFETY: No entries were added or removed.
        .unwrap()
}

pub fn try_map<T, E>(
    self,
    mut f: impl FnMut((&K, V)) -> Result<T, E>,
) -> Result<BoundedBTreeMap<K, T, S>, E> {
    Ok(self
        .0
        .into_iter()
        .map(|(k, v)| (f((&k, v)).map(|t| (k, t))))
        .collect::<Result<BTreeMap<_, _>, _>>()?
        .try_into()
        // SAFETY: No entries were added or removed.
        .unwrap())
}
@github-actions github-actions bot added the J2-unconfirmed Issue might be valid, but it’s not yet known. label Jul 19, 2022
@benluelo benluelo changed the title iter_mut/ map_in_place for BoundedBTreeMap map/try_map for BoundedBTreeMap Jul 19, 2022
@shawntabrizi
Copy link
Member

Sounds reasonable. Would you like to work on this?

@shawntabrizi shawntabrizi added J0-enhancement An additional feature request. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. and removed J2-unconfirmed Issue might be valid, but it’s not yet known. labels Jul 19, 2022
@benluelo
Copy link
Contributor Author

Absolutely! I will open a PR shortly 👍

@benluelo
Copy link
Contributor Author

#11869 implemented similar behaviour to this (adding an iter_mut() method to the Bounded* collections), but as per my comment (#11869 (comment)) I still think that this would be useful functionality to have. I often create one bounded collection from another with the same length, which results in unsightly .expect(...)s throughout the code.

@ggwpez
Copy link
Member

ggwpez commented Oct 29, 2022

One issue with iter_mut is that it doesn't allow you to transform the item's type in the collection […]

Yea… I guess that is a valid concern. Do you want to implement map/try_map as well?

@benluelo
Copy link
Contributor Author

Yeah I can definitely do that! (#11869 actually started out as that 😅) I'll open a new PR for it 👍

@ggwpez
Copy link
Member

ggwpez commented Oct 29, 2022

I often create one bounded collection from another with the same length, which results in unsightly .expect(...)s throughout the code.

FYI there is defensive_truncate_from which can at-least get rid of these expects.

Repository owner moved this from Backlog to Done in Runtime / FRAME Nov 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants