Rename `Event` to `Mutation`
eimantas opened this issue · 23 comments
What was the motivation behind this rename as the version change minor, but this change broke the source backwards compatibility.
Not sure if @kzaher or @freak4pc have another answer but I think the main reason is that event is hard to reason about specially when you're new to the concept. Mutation is much readable just because the main purpose of the event is to mutate the state so why not call it mutation?
I agree though that the changes are not backward compatible.
I think it’s fair to say that if we follow semver this should’ve been a major version bump because it breaks backwards compatibility. Given it’s a relatively young framework and a relatively small change I’m not sure we should at this point but this is up to @kzaher of course.
We could also have a @deprecated version of Event but again, since this framework is
Relatively young I think that would be counter productive.
@eimantas We can add typealias if you like.
It is really hard to explain this concept to people. I understand that it could cause some breakage, but shouldn’t be that horrible.
Changing the name from Event, which is one way of looking at it, to mutation, which is another more concrete way seems to lift some cognitive load and document the intent of it better.
Apple is causing rename breakages with minor versions of Swift and people don’t seem to make fuss of it.
Since this is a small library and it seems that the rename helps people, seemed ok.
In case this is a small and young library, I believe that the release of 1.0 (let alone 1.1) was rushed. It could have matured quite a bit. Because usual libraries that reach 1.0 have stable APIs. And even though it's small library it's not used for some specific purpose, but to build entire application's architecture. So no matter how small the breaking change, users would have to go through entire codebase to fix the breakage.
Regarding the rename, IMHO this completely doesn't make sense to me. It was very clear when I read the documentation first what event is for and how state is manipulated. The Mutation as a term (again, IMHO) means the delta of the state (old values vs new values) while in reality (and in code) the mutation itself is performed in state's reducer.
Also in code the events that go into Bindings initializer cannot be closer to the definition of the event. Because these are UI events: button taps, row selections, slider adjustments, etc. The mutation happens in other layers of abstraction (State as Model vs Button as View). As a (somewhat) contrived example: if I pass an rx.tap mapped to Event.refreshData it reads far better as event being sent rather than a mutation. Will mutation in this case be nilling out existing data and fetching new one or keeping the old one until the new arrives? As you can see this is business logic and should be in reducer and the event should be freed of this knowledge completely.
The Mutation also implies necessity to mutate the state which not always may be the case (although 99.99% of the times it will).
So for me it's not a clearer definition of what I'm passing into the Bindings. If there were a proposal regarding this rename, I'd vote 👎 .
In any case if majority of beginners find it better - I appreciate the work that you do on this library for wider adoption 🙇
P.S. to be fair in some cases (maybe even more often than not) there could be no mutation because under the hood a completely new state would be created. ¯_(ツ)_/¯
And even though it's small library it's not used for some specific purpose, but to build entire application's architecture. So no matter how small the breaking change, users would have to go through entire codebase to fix the breakage.
Were there any concrete issues you've experienced as a result of this change?
Were they bigger then the time you've spent updating your projects to support Xcode 10?
In case this is a small and young library, I believe that the release of 1.0 (let alone 1.1) was rushed. It could have matured quite a bit. Because usual libraries that reach 1.0 have stable APIs.
I guess I didn't see how this change could cause issues although the public API is different. Maybe I've overlooked something obvious. Can you provide some concrete examples? I wouldn't say that 1.0 was rushed. I've spent 2 years using this privately and thinking about this concept, but there will always be improvements that can be made.
Regarding the rename, IMHO this completely doesn't make sense to me. It was very clear when I read the documentation first what event is for and how state is manipulated. The Mutation as a term (again, IMHO) means the delta of the state (old values vs new values) while in reality (and in code) the mutation itself is performed in state's reducer.
That's great. It was perfectly clear for me also since I've decided on the terminology and it fits FSM definitions https://en.wikipedia.org/wiki/Finite-state_machine#State/Event_table. Unfortunately this isn't significant statistical sample.
The Mutation also implies necessity to mutate the state which not always may be the case (although 99.99% of the times it will).
I agree. But I'm also flexible enough to redefine my view of the world and say if the reducer is called with no op mutation, it still mutated the storage and replaced it with a new equal value. This is what actually happens. Wouldn't it actually be a code smell to think of a value being mutated since values are immutable by definition?
In any case if majority of beginners find it better - I appreciate the work that you do on this library for wider adoption 🙇
We'll see. For some inexplicable reason people find it hard to grasp and I can't figure out why. There is a real 2.0 version that has a bit simpler implementation and potentially a bit uglier public interface that I'm considering of releasing, but it's conceptually equivalent.
This:
Also in code the events that go into Bindings initializer cannot be closer to the definition of the event. Because these are UI events: button taps, row selections, slider adjustments, etc.
Is basically not right - a feedback loop doesn't have to be fed by UI elements.
Also, Mutation really means "an intent to mutate the state - and I also find it more readable and understandable, basically, when compared to Event (which is basically like thinking of "a mutation event").
Is basically not right - a feedback loop doesn't have to be fed by UI elements.
Does not have to be, but most of the times (at least in projects I work with) is. But to be fair - I wouldn't dismiss wrong usage pattern here.
Also, Mutation really means "an intent to mutate the state - and I also find it more readable and understandable, basically, when compared to Event (which is basically like thinking of "a mutation event").
This is probably where we will disagree. Mutation can be direct (change state through reducer) and indirect (create a query that triggers a network request). It can also be a side-effect (i.e. a mutation outside of it's controlled domain, like logging to a file) that is reset right after the next event.
Were there any concrete issues you've experienced as a result of this change?
Were they bigger then the time you've spent updating your projects to support Xcode 10?
No and no, but on the big project it stacks up. Especially given that one screen (VC) can have multiple bindings coming from different UIView subclasses. So basically if I sounded annoyed and/or displeased is because this came more as an unexpected change that had me clicking Cmd+Opt+Ctrl+F on quite a few lines in compilation warnings pane (while migrating the project to Xcode 10 and Swift 4.2).
I guess I didn't see how this change could cause issues although the public API is different. Maybe I've overlooked something obvious. Can you provide some concrete examples?
Deprecation notice would have been nice. When I started using the library it had deprecation methods for system bootstrapping methods and the way feedback loops were passed. So I knew what to use upfront. This one comes like thunder from clear skies.
That's great. It was perfectly clear for me also since I've decided on the terminology and it fits FSM definitions https://en.wikipedia.org/wiki/Finite-state_machine#State/Event_table. Unfortunately this isn't significant statistical sample.
This probably could have been addressed through documentation (i.e. adding the origin terminology). There could be the possibility that newcomers are finding it hard to understand/adopt because they're not familiar with FSMs.
Hi all 👋
system is a lovely operator that makes reactive programming a breeze. It profoundly changed the way I think about using a reactive approach in a stateful environment. Thank you 🎉
I'm sure that you have thought this through better than anyone else. Though, in my experience using the system operartor an Event rename to Mutation suggests a responsibility shift from reduce to feedback. Let me elaborate 😉
Let's look at the example given in the README:
Driver.system(
initialState: 0,
reduce: { (state: State, mutation: Mutation) -> State in
switch mutation {
case .increment:
return state + 1
case .decrement:
return state - 1
}
},
feedback:
bind { state in
([
state.map(String.init).bind(to: label.rx.text)
], [
plus.rx.tap.map { Mutation.increment },
minus.rx.tap.map { Mutation.decrement }
])
}
)This example makes perfect sense! reduce applies the mutation remaining totally stupid. The feedback, on the other hand, is being smart, it knows the state very well and when a tap event happens it decides to pass it as a increment / decrement mutation.
Now let's imagine that we don't want to go bellow 0 and see what happens 💭
We have two places were we can put this logic:
- We can put it in
feedback:
plus.rx.tap.map { Mutation.increment },
minus.rx.tap.withLatestFrom(state).map { $0 > 0 ? Mutation.decrement : nil }.filterNil()Keeping the logic in one place makes the feedback even smarter and knowing about the insides of state even better. The code became harder to read, different feedback loops have to make sure they don't contradict the logic of others. Somehow, writting such code doesn't feel right 😞
- We can put it in
reduce:
switch mutation {
case .increment:
return state + 1
case .decrement:
return max(0, state - 1)
}That would make reduce smart instead of stupid, meaning the logic would be spread out in two places having two smart parts operating together. It's a bit hard to reason about the overall functionality of the system as it's chopped in pieces. Worse of all, the reduce is not respecting the mutation it was ordered to apply. Somehow, this doesn't feel right as well 😞
@kzaher, @freak4pc, I wonder, how would you implement this?
Maybe there's a third option with Mutation that I didn't think of? 🤔
If we use Event, it's obvious how to do this 🌈
3. We just put all the logic in reduce:
Driver.system(
initialState: 0,
reduce: { (state: State, event: Event) -> State in
switch event {
case .tappedPlus:
return state + 1
case .tappedMinus:
return max(0, state - 1)
}
},
feedback:
bind { state in
([
state.map(String.init).bind(to: label.rx.text)
], [
plus.rx.tap.map { Event.tappedPlus },
minus.rx.tap.map { Event.tappedMinus }
])
}
)Now this is the magic that made me fall in love with RxFeedback 😍 It moves all of the logic to reduce making it super smart, but that's ok! Because reduce is a pure function that is really easy to reason about and to test if we want to ✅
All feeedback loops become super stupid, they just pass the event that happened without thinking anything about the state and how would it change after the event. And it's nice, because there's a single job that's difficult enough to be on its own.
There's a great talk by Andy Matuschak that aligns with the 3rd option very well. He there introduces an idea:
Make the imperative shell just thin glue; the functional core pure and heavily tested; they communicate by message passing.
I wonder what are your thoughts about this responsibility shift that I find from reduce to feedback, once we change the name from Event to Mutation.
Thank you for the amazing work 🙇
This is awesome!
There's nothing wrong with your third option but, in my honest opinion, the issue is you're thinking of Event as a UI Event, instead of describing your intent to create some mutation, or really the intent to do some action (similar to redux).
Event.tappedPlus
Event.tappedMinus Might be better named as
Mutation.increment
Mutation.decrementLike in your 2nd example. That doesn't mean the implementation you mentioned on that 2nd clause is the correct one.
tl;dr I don't see anything wrong your third option, while still using the Mutation.increment/decrement naming, instead. This is also more general-use, and doesn't reflect just "Events:" (be it user-triggered or not), but the intent for changing something in your state, which to me - feels clearer.
RxFeedback is made by one person, thus, its opinionated. This is all down to Kruno's opinion, I merely made the PR to help and support his spare time :) But I still feel its a significantly better and clearer name. Intent is also nice (like MVI - Model View Intent), but I wouldn't want to just go for another rename for no apparent reason ATM.
From how I understand the concept it seems to me that Event is a perfect name for what it does (IMHO). As Event essentially notifies a system that something happened and it's up to the reducer to decide whether there should be any changes to the State as you may design your system to ignore some events when it is in particular state ¯_(ツ)_/¯
This probably could have been addressed through documentation (i.e. adding the origin terminology). There could be the possibility that newcomers are finding it hard to understand/adopt because they're not familiar with FSMs.
We can, but the expectation that people will take time to read the docs isn't realistic IMHO.
@domasn The intention for the rename wasn't to change the concept. I'm not sure you examples are complex enough to illustrate any points.
I would say first that example with splitting the logic would be against my intuition and intended use of this library.
Reducer should contain all testable logic.
Keeping the logic in one place makes the feedback even smarter and knowing about the insides of state even better. The code became harder to read, different feedback loops have to make sure they don't contradict the logic of others. Somehow, writing such code doesn't feel right 😞
Adapter logic inside feedback loops should be primitive data transformations, nothing else. Not something like $0 > 0 ? Mutation.decrement : nil for sure.
Event is an overloaded term.
It can be used in the context of state machines to describe "input".
It can be used in the context of UI to describe "output".
In the end, it doesn't tell you anything semantically useful except that it describes a concept with no duration. Something PublishSubject like. It represents a distinct point in time between state transitions. Point of breakage in the continuum.
Mutation is more specific. It says. I'm creating this structure with the intent of possibly changing the state. There is no ambiguity are mutations input or output. They are input in my book.
Since mutation application represents transition between two discrete states, I believe this is exactly what that data structure is.
Regarding naming.
As far as I'm concerned, you can call it.
Mutation.tappedPlus
Mutation.increment
Mutation.goUp
Mutation.someEventHappened
...
or any other name. Some of my coworkers have more religious opinions about this, but in the end it doesn't matter much.
If you read .tappedPlus, .increment, .goUp ... it's not that hard to figure out why this thing exists.
If you take an example with button click and incrementing, there is an "output" event that is being observed on button (when state transitions from tapped to not tapped), and that "event" is being used to create input event ("Mutation") that will transition state inside system operator and do +1.
Maybe this change looks sudden externally, but it does make sense in my book.
Having said that, if people will again be confused, then I'll switch it back. If I knew how to predict people herd behavior I would be a market speculator.
In my view the term Mutation are more unambiguous and clear than Event in order to understand the RxFeedback concept.
Btw, I've found even a better term for a mutation event — command. Command means exactly what we are talking about — "a structure with the intent of possibly changing the state".
@ixrevo it could work, but mutation seems like something any swift user should understand without familiarity with gof patterns.
I'm doing cleanups and generalizations. I've renamed Query to Request. Changes are in the master branch. I guess now is the time to comment :) I'm thinking about releasing 2.x version.
Great rename! Request seems more familiar than Query.
A few more thoughts regarding Event vs Mutation.
All developers that I know who have tried using the system operator (~20 people) struggled at the same place:
Straightforward
if it's state -> State
if it's a way to modify state -> Mutation/Command
it it's an effect -> encode it into part of state and then design a feedback loop
This part is definitely not straightforward for most 😄 The hardest part of using system to solve a problem is separating mutations/commands from side effects.
Let's look at fairly simple example of GithubPaginatedSearch. The example lists startLoadingNextPage as one the mutation that is sent to reducer when table is near the bottom. That get's turned into loadNextPage property that triggers the side effect of making an networking call.
This is not an easy solution to figure out. Why? I think, because, when sending the mutation to reducer we already know what side effect should be done, but we don't do in place (as it's common in FRP), because we're not sure what page to load next if any. Such solution is counter-intuitive - is "load next page" thing a mutation or a side effect? Weirdly, in this case the answer is both - a mutation startLoadingNextPage and a side effect (encoded into state as loadNextPage) 🙀
In my experience, what helps people to easily separate mutations from side effects is this:
- If it did happen -> Event
- If it should happen -> Request
- To fulfill Request -> Feedback loop
That's it! 🎉
With this in mind, we could easily list all events and requests of GithubPaginatedSearch:
enum Event {
// All events just happened, so we use past-simple tense verbs
case searchChanged(String)
case gotNearBottom
case loadedNextPage(SearchRepositoriesResponse)
}
// Requests
extension State {
// Requests should happen, so we use imperative verbs
var loadNextPage: URL? {
return self.shouldLoadNextPage ? self.nextPageURL : nil
}
}This clearly puts reduce in a decision-making position, you tell it what happened, it tells you what to do next. And that's what we want - give all the power to the pure function!! 👑
I think, "event" is a better word for something that just happened, no hidden intentions as in "mutation".
Hi @domasn ,
All developers that I know who have tried using the system operator (~20 people) struggled at the same place:
This is an interesting feedback. Usually everybody I've tried to explain this just says "ha?" as a first reaction :)
Such solution is counter-intuitive - is "load next page" thing a mutation or a side effect? Weirdly, in this case the answer is both - a mutation startLoadingNextPage and a side effect (encoded into state as loadNextPage) 🙀
I've noticed that the provided example is in fact confusing. It had startLoadingNextPage in the mutation. I've changed the example to this code:
fileprivate enum Mutation {
case searchChanged(String)
case response(SearchRepositoriesResponse)
case scrollingNearBottom
}This clearly puts reduce in a decision-making position, you tell it what happened, it tells you what to do next. And that's what we want - give all the power to the pure function!! 👑
Yes, this is what you want, I agree.
I think, "event" is a better word for something that just happened, no hidden intentions as in "mutation".
I guess I'm fine on using the Event again vs the Mutation if that helps.
In my experience, what helps people to easily separate mutations from side effects is this:
If it did happen -> Event
If it should happen -> Request
To fulfill Request -> Feedback loop
This is a really clear and intuitive way of describing this, IMO. If this accurately represents a rule of thumb, I think it would even be valuable to have it as part of the README.
Thoughts?
I think, "event" is a better word for something that just happened, no hidden intentions as in "mutation".
I guess I'm fine on using the Event again vs the Mutation if that helps.
@freak4pc what do you think about going back to "Event"?
This isn’t my project :) I’m just helping out like the rest of you.
I’m not sure if it’s smart switching names because of a single opinion. If there is any group of people that are more avid users of RxFeedback, it might make more sense to loop these in to see what they think of the terminology.
🎆