Skip to content

Switch SideEffects to have any input be apart of their function #13

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

Closed
GregoryConrad opened this issue Dec 13, 2023 · 4 comments
Closed

Comments

@GregoryConrad
Copy link
Owner

GregoryConrad commented Dec 13, 2023

As an alternative to #3, we could perhaps rewrite side effects to use a form of fn as_listener(register: SideEffectRegistrar, _: ()) {}.

Then, you would do register((as_listener, ())), or register((effects::state, 1234), (effect2, ())).

Not sure which approach I like better. Current approach requires more code indentation (returning a closure), but this one is forced to one side effect argument at all times, and requires wrapping everything in tuples.

@GregoryConrad
Copy link
Owner Author

Too unergonomic for actual use. Easier to write side effects, but writing side effects is on the rarer side, especially for the Rust implementation.

@GregoryConrad GregoryConrad added the wontfix This will not be worked on label Dec 13, 2023
@GregoryConrad
Copy link
Owner Author

GregoryConrad commented Dec 13, 2023

I suppose we could also try to support an API like:

let ((state, set_state), rebuilder)= registrar
    .register(effects::state, 1234)
    .register(effects::rebuilder, ())
    .complete()

And then for one side effect we can add a convenience registrar.single(state, 1234).

Might be able to integrate with the tuple_list crate to make such an API not a total pain to implement.

@GregoryConrad GregoryConrad reopened this Dec 13, 2023
@GregoryConrad GregoryConrad removed the wontfix This will not be worked on label Dec 13, 2023
@GregoryConrad
Copy link
Owner Author

We could also ship a proc macro for side effects that allows us to do:

let ((state, set_state), _) = register.state(1234).as_listener().complete();

To me, this is the best of all worlds. Won't require better-api feature gate, method chaining is declarative and readable, and GATs should no longer be an issue. Might take a crack at implementing this.

@GregoryConrad
Copy link
Owner Author

Changed my mind; this is too unergonomic and is hard to implement. It is still easier to just write out register.register((effect1(), effect2(1234))) on stable rust than this alternative is, and that API makes the impl far far easier than this one would be.

@GregoryConrad GregoryConrad closed this as not planned Won't fix, can't repro, duplicate, stale Dec 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant