Skip to content
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

Closed
wants to merge 11 commits into from

Conversation

inamiy
Copy link
Contributor

@inamiy inamiy commented Jul 1, 2019

This proposal introduces Lens and Prism to break down a large ReactiveFeedback reducer / feedback / state / event into small pieces.

Please see @mbrandonw 's talk for more detail:
Brandon Williams - Composable Reducers & Effects Systems - YouTube

@inamiy inamiy requested a review from a team July 1, 2019 16:40
@inamiy inamiy self-assigned this Jul 1, 2019
@ghost ghost requested review from anilputtabuddhi, mr-v and TheAdamBorek and removed request for a team July 1, 2019 16:40
}
}

let mainReducer: Reducer<MainAction, MainState> =
Copy link
Contributor

@ilyapuchka ilyapuchka Jul 1, 2019

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 the MainState (which is yet to be defined by the lens/prism implementations), right?

Is it different essentially from breaking reducer into smaller functions?

func reducer(state: State, event: Event) -> Event {
  switch event {
    case .sub1(sub1Event):
      return state.set(\.sub1, reduceSub1(state: state.sub1, event: sub1Event))
    ...
  }
}

func reduceSub1(state: Sub1State, event: Sub1Action) -> Sub1State {
  switch event { ... }
}

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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. 👍

Copy link
Contributor

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

Copy link
Contributor Author

@inamiy inamiy Jul 3, 2019

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.

Copy link
Contributor

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 👍

Copy link
Contributor Author

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.

@inamiy
Copy link
Contributor Author

inamiy commented Jul 2, 2019

4db5676
Added "Composable Feedbacks" example using Lens and Prism, which strengthens the importance of these functional approaches compared to manually writing to repeat the same main code over and over again, cf. #171 (comment)

inamiy and others added 2 commits July 2, 2019 18:06
Co-Authored-By: Ilya Puchka <ilyapuchka@gmail.com>
Co-Authored-By: Ilya Puchka <ilyapuchka@gmail.com>
Co-Authored-By: Giorgos Tsiapaliokas <giorgos.tsiapaliokas@mykolab.com>

/// For accessing enum cases.
/// e.g. Whole = all possible enum cases, B = partial case
struct Prism<Whole, Part> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are preview and review standard lingo for a Prism? I know that Brandon used these names in his presentation but I don’t think they describe the purpose of the two functions very well. To me, subspace and update reads more clearly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Brandon followed Haskell's naming for those: http://hackage.haskell.org/package/lens-4.17.1/docs/Control-Lens-Prism.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zzcgumn
Yes, preview and review are commonly used names in Haskell.

There are some alternatives for Prism method names which may be more intuitive:

  1. Scala Monocle uses getOption: Whole => Option[Part] and reverseGet (aka apply): Part => Whole
    https://julien-truffaut.github.io/Monocle/optics/prism.html

  2. tryGet: Whole -> Part? and inject: Part -> Whole is introduced here
    https://broomburgo.github.io/fun-ios/post/lenses-and-prisms-in-swift-a-pragmatic-approach/

I'm OK with renaming too (I personally prefer 2.), but we might want to stay as is, since there are many more upcoming layers in these functional Optics family as described in
http://oleg.fi/gists/posts/2017-04-18-glassery.html

So renaming each of them may probably consume our brain too much that it's often better to just borrow the same names from original.

@zzcgumn
Copy link
Contributor

zzcgumn commented Jul 8, 2019

Could you please provide an example that illustrates why this approach is preferable compared to defining a couple of SubViewModels?

I can see that this proposal adds good value if it allows us to create more of a plug and play architecture where the app the content is effectively calculated at runtime. Examples could be building a tab bar dynamically by something like (AppConfiguration, ConsumerNetwork, UserData) -> [Tabs] or dynamic screen content (AppConfiguration, ConsumerNetwork, UserData) -> [Cards]. What I am not sure about is why defining Prism and Lens instances makes this easier compared to MainViewModel and SubViewModel.

@inamiy
Copy link
Contributor Author

inamiy commented Jul 9, 2019

Filled in missing documents in 9122ca4 & 47bde8b , so it's ready to review 🙂

```swift
let reducer: Reducer<Action, State> =
sub3Reducer
.lift(action: .sub3Event >>> .sub2Event >>> .sub1Event)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't you think though that this will make debugging more complicated as it will add extra steps to reach the sub reducer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyapuchka
Not sure why you feel it's harder to debug 🙄
Do you want to add a breakpoint in the intermediate reducer that never appear in this code?
I think adding breakpoints in sub3Reducer is sufficient because we aren't interested in intermediate reducers at all.

@inamiy
Copy link
Contributor Author

inamiy commented Jul 19, 2019

cf. https://github.com/inamiy/FunOptics for lightweight Optics library.

@mr-v mr-v removed their request for review August 2, 2019 08:45
@RuiAAPeres
Copy link
Contributor

Closing due to inactivity. Please re-open when you have time to work on it. ❤️

@RuiAAPeres RuiAAPeres closed this Aug 20, 2019
@inamiy
Copy link
Contributor Author

inamiy commented Aug 20, 2019

@RuiAAPeres
I'm guessing it's highly active by @pointfreeco https://www.pointfree.co/episodes/ep70-composable-state-management-action-pullbacks 🙂

Jokes aside, I will reopen this when more huge RAFs become needed in our project.

@inamiy
Copy link
Contributor Author

inamiy commented Oct 21, 2019

cf. https://github.com/inamiy/Harvest-SwiftUI-Gallery (Example App)

Here's my take for Composable Architecture using Optics (Lenses and Prisms) which this PR is proposing.

@RuiAAPeres
Copy link
Contributor

Jokes aside, I will reopen this when more huge RAFs become needed in our project.

I need this on "my" project. ❤️

Jokes aside, if there's no interest in merging this back to the official RAF, I will happily use whatever you come up with in your own side project (as long it uses RAS as base). 😉

@inamiy
Copy link
Contributor Author

inamiy commented Nov 2, 2019

@RuiAAPeres
Using Optics is rather a team's coding convention, so it doesn't necessary have to belong as part of RAF.
(Of course it will be nicer if these helpers are officially provided, just as I made HarvestOptics as a separate framework but in the same repo)

Also, choosing what Optics library to use is another topic to consider.
IMO to keep them as separate as possible, those combinations can be achieved in your own project side 👍

@inamiy
Copy link
Contributor Author

inamiy commented Nov 2, 2019

@RuiAAPeres
Copy link
Contributor

RuiAAPeres commented Nov 3, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants