Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure Index trait to allow for more extensive Index struct implementation. #757

Closed
y-pakorn opened this issue Jul 20, 2022 · 1 comment

Comments

@y-pakorn
Copy link
Contributor

This should not affect/or break anything. Trait change allows for the implemented Index struct can access both old_data and data and act accordingly.

For the ideas, In some cases, we will not need to update the indexes every update, only handle the first save and remove. This can be called ConstantMultiIndexes.

Or we can update the indexes whether the condition of the data updated is met. This can be called ConditionalMultiIndexes.

pub trait Index<T>
where
    T: Serialize + DeserializeOwned + Clone,
{
    fn update(
        &self,
        store: &mut dyn Storage,
        pk: &[u8],
        old_data: Option<&T>,
        data: Option<&T>,
    ) -> StdResult<()>;

    fn save(&self, store: &mut dyn Storage, pk: &[u8], data: &T) -> StdResult<()> {
        self.update(store, pk, None, Some(data))
    }

    fn remove(&self, store: &mut dyn Storage, pk: &[u8], old_data: &T) -> StdResult<()> {
        self.update(store, pk, Some(old_data), None)
    }
}
@maurolacy
Copy link
Contributor

For what I can see in the associated PR, this is just cosmetics: Moving replace's functionality inside of update, and handling both save and remove in terms of update.

I wouldn't merge this unless there's a good reason that indicates this is better than the current implementation.

@maurolacy maurolacy changed the title [cw-storage-plus] Restructure Index trait to allow for more extensive Index struct implementation. Restructure Index trait to allow for more extensive Index struct implementation. Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants