Make Command trait API more functional by eliminating mutable parameters #22

New issue

Closed

opened 2025-07-06 08:36:22 -07:00 by jwilger · 1 comment

jwilger commented

2025-07-06 08:36:22 -07:00

(Migrated from github.com)

Context

During the refactoring of executor.rs in PR #10, it was noted that the Command trait's handle method requires a mutable reference to StreamResolver, which goes against functional programming principles.

Current API

async fn handle(
    &self,
    read_streams: ReadStreams<Self::StreamSet>,
    state: Self::State,
    stream_resolver: &mut StreamResolver,  // <-- mutable reference
) -> CommandResult<Vec<StreamWrite<Self::StreamSet, Self::Event>>>;

Proposed Functional API

A more functional approach would return the modified StreamResolver along with the events:

async fn handle(
    &self,
    read_streams: ReadStreams<Self::StreamSet>,
    state: Self::State,
    stream_resolver: StreamResolver,  // <-- owned value
) -> CommandResult<(Vec<StreamWrite<Self::StreamSet, Self::Event>>, StreamResolver)>;

Or use a more structured return type:

struct CommandOutput<SS, E> {
    events: Vec<StreamWrite<SS, E>>,
    stream_resolver: StreamResolver,
}

async fn handle(
    &self,
    read_streams: ReadStreams<Self::StreamSet>,
    state: Self::State,
    stream_resolver: StreamResolver,
) -> CommandResult<CommandOutput<Self::StreamSet, Self::Event>>;

Benefits

Immutability: No mutable references, making the code more predictable and easier to reason about
Functional composition: Easier to chain operations and build pipelines
Thread safety: Immutable data is inherently thread-safe
Testing: Pure functions with inputs and outputs are easier to test

Considerations

This would be a breaking change to the public API
All existing command implementations would need to be updated
Since we're still in 0.x.x versions, this is acceptable

See PR #10 review comments: https://github.com/jwilger/eventcore/pull/10#discussion_r1924574630

## Context During the refactoring of `executor.rs` in PR #10, it was noted that the `Command` trait's `handle` method requires a mutable reference to `StreamResolver`, which goes against functional programming principles. ## Current API ```rust async fn handle( &self, read_streams: ReadStreams<Self::StreamSet>, state: Self::State, stream_resolver: &mut StreamResolver, // <-- mutable reference ) -> CommandResult<Vec<StreamWrite<Self::StreamSet, Self::Event>>>; ``` ## Proposed Functional API A more functional approach would return the modified `StreamResolver` along with the events: ```rust async fn handle( &self, read_streams: ReadStreams<Self::StreamSet>, state: Self::State, stream_resolver: StreamResolver, // <-- owned value ) -> CommandResult<(Vec<StreamWrite<Self::StreamSet, Self::Event>>, StreamResolver)>; ``` Or use a more structured return type: ```rust struct CommandOutput<SS, E> { events: Vec<StreamWrite<SS, E>>, stream_resolver: StreamResolver, } async fn handle( &self, read_streams: ReadStreams<Self::StreamSet>, state: Self::State, stream_resolver: StreamResolver, ) -> CommandResult<CommandOutput<Self::StreamSet, Self::Event>>; ``` ## Benefits 1. **Immutability**: No mutable references, making the code more predictable and easier to reason about 2. **Functional composition**: Easier to chain operations and build pipelines 3. **Thread safety**: Immutable data is inherently thread-safe 4. **Testing**: Pure functions with inputs and outputs are easier to test ## Considerations - This would be a breaking change to the public API - All existing command implementations would need to be updated - Since we're still in 0.x.x versions, this is acceptable ## Related Discussion See PR #10 review comments: https://github.com/jwilger/eventcore/pull/10#discussion_r1924574630

emberian commented

2025-10-08 18:16:57 -07:00

(Migrated from github.com)

I'd like to push back on this. mutable references can be a beautiful part of a functional API. in purely functional languages we have no choice but to take-by-value and return a new copy "writer pattern". but in rust when we're able to use a &mut this is exactly the same as if using the "writer" pattern. basically I don't believe the "benefits" because the code can be written equivalently both ones, one is just slightly friendly to the compiler. to phrase another way, "please don't replace rust's unique language feature with an ugly writer monad"

I'd like to push back on this. mutable references can be a beautiful part of a functional API. in purely functional languages we have _no choice_ but to take-by-value and return a new copy "writer pattern". but in rust when we're able to use a `&mut` this is exactly the same as if using the "writer" pattern. basically I don't believe the "benefits" because the code can be written equivalently both ones, one is just slightly friendly to the compiler. to phrase another way, "please don't replace rust's unique language feature with an ugly writer monad"