Comments/Feedback on crate idea

code review
1

Hi all,

I recently whipped up a little crate called EffectCell which runs an effect whenever the data inside is updated, like so:

use effect_cell::EffectCell;

fn main() {
    let mut fxl = EffectCell::new(1, |x| println!("{x}"));
    fxl <<= 2; // prints 1
    fxl <<= 4; // prints 2
}

I was wondering if anyone has any use cases for a type like this; I was thinking it would be useful for debugging. Or maybe it could be useful for resource management or deferring some kind of action . . . I don't know yet :thinking:

Also, does anybody have feedback for API design/internals? Would it be more useful as a type with interior mutability? Would if be more useful if it ran the effect on the new data on every mutation (instead of the old data)? etc.

The code is on github, and an initial release is on crates.io.

Thanks for your help everyone!

PS: If you think this crate is interesting, you might find the joe crate interesting :).

2 Likes
2

Its an interesting concept for sure. In essence, a stripped down version of the observer pattern with a singular localised callback.

Looking into the codebase, you've triggered one of my pet peeves with idiomatic design. Assign unrelated functionality to internal operators. The use of ShlAssign is unnecessary and contains the functionality that should have been included in update. Short-hands like this are rarely helpful due to editor tools and auto complete making calling update one .u tab away. What would be better is to extend these operations to apply to data stored within.

impl<T, E> AddAssign for EffectCell<T, E>
where
    T: AddAssign 
{
    fn add_assign(&mut self, rhs: T) {
        self.data += rhs;
        self.call();
    }
}

Operator overloading that doesn't represent the intended functionality can cause issues when functions and structs with trait bounds come into play.

Also what is the purpose of the Index trait impl?
index_mut appears to run the effect on the creation of a &mut before any data has been modified. This is redundant and not the intended functionality of IndexMut.

The update function calls the effect before updating state.

self.call();
self.data = rhs;

Usually in the observer pattern, the purpose is to run some code after state changes so that actions can be performed considering what has changed, rather than operating on the old state.

self.data = rhs;
self.call();

In general, this is a good start and could be further evolved to support multiple callbacks on the same structure, with varying execution positioning (before or after assignment).

5 Likes
3

Thanks for the response!

The overloading is mainly a cosmetic thing - I'm not too attached to it.

Looking back on the indexing, I'm not sure what that was about :thinking: probably why we shouldn't program late at night :stuck_out_tongue:.

Making this have multiple variants for different execution times and multiple callbacks seems useful - I'll implement that at some point.

Once again, thanks for the feedback!

I hope this can be a useful type :slight_smile:

4

I ended up trying to do some of these updates myself.
The code is on GitHub: GitHub - ScratchCat458/effect_cell: Stripped-down observer implementation using closures
I have an Action setup for tests and doc generation

2 Likes
5

That looks really good! Would you want to submit a pull? No pressure - especially if you have more ideas.

6

Sorry for the late reply!
I've written some custom Debug impls for the Effect Cell types and made a pull request to your repo on GitHub. I have also updated the README to cover the type signature changes and pushed an action for running doctests, format checking and lints.
main.rs was removed as it didn't have much purpose and doctest are much more helpful.