Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Proposal] Composable Reducers using Lenses and Prisms #171
[Proposal] Composable Reducers using Lenses and Prisms #171
Changes from 1 commit
0f39e24
5390a38
4db5676
77b8ebc
ac5a715
5e5512c
606ab60
3d4c3f1
9122ca4
47bde8b
9b5188d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this what will happen with the
mainReducer
? It will call the sub reducers to take care of their respective parts and then translate their results into theMainState
(which is yet to be defined by the lens/prism implementations), right?Is it different essentially from breaking reducer into smaller functions?
Maybe seeing how this will fit into splitting view models (if it will) and with feedbacks will make this difference more appealing than for just reducers on their own?
Comparing two different approaches side by side will also probably make the difference more visual.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By introducing "Composable Reducers", we don't have to write
reducer
(main reducer) pattern-matching, which we don't want to repeat things over and over again.I also added "Composable Feedbacks" example in 4db5676 , so it becomes more apparent if we rewrite
mainFeedback
by hand.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd emphasise even more this point in the proposal itself, being one of the most powerful advantages of FP compositionality. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But don't we write the same pattern matching in the prisms implementations? =) It might be a bit easier to code generate, but until it is in place we'll have to write this boilerplate 🤷♂ I like how prisms look simple and separate concerns and I used them (in a bit different form) myself, but not sure what boilerplate will be more clear.
P.S. I'm 💯 into code generating this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ilyapuchka
Pattern-matching in small
Prism
layer and making its composition is more FP way than pattern-matching whole gigantic enum at once filled with un-reusable code.It becomes more and more apparent once we have n-layered reducers / feedbacks.
(Though it may sound unrealistic, IMO we should be ready for such scalable architecture)
And I totally agree that this proposal gains more popularity once we introduce codegen.
But even without it, I think it's worth introducing
Lens
/Prism
for scalability.I personally can't live without these features to split large codebase.
(Currently ongoing in https://github.com/Babylonpartners/babylon-ios/pull/7938 )
And unfortunately, current Swift
KeyPath
can't solve this problem.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm all into splitting huge switch into smaller pieces, that should be probably emphasised in the proposal as a win with pros and cons of each approach (I'm not yet sold thought that it will improve reusability on practice due to how types depend on each other in our code base) 👍
I think it will worth also adding your thoughts about why KeyPaths don't fit in the proposal, good candidate for Alternatives Considered section 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
47bde8b Added composition example to explain the simpler solution.