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

Add "Distinct" property wrapper #200

Closed

Conversation

haeseoklee
Copy link

@haeseoklee haeseoklee commented Mar 20, 2022

Motivation

Observer subscribing to the reactor state causes unnecessary view re-rendering when the distinctUntilChanged operator is not applied.

class TestReactor: Reactor {
  
  struct State {
    var items: [String] = ["a", "b", "c"]
  }
}
class TestViewController: UIViewController, View {

  func bind(reactor: TestReactor) {
    reactor.state
      .map { $0.items }
      .distinctUntilChanged()
      .subscribe(onNext: { items in
        // πŸ€” If distinctUntilchanged is not applied, there is a problem that duplicate data comes in.
        // And it is not compulsory and may be omitted by mistake.
      })
      .disposed(by: disposeBag)
  }
}

Therefore, in order to prevent unnecessary re-rendering, this problem must be prevented during the property declaration phase. This idea was inspired by Pulse. Thanks to @tokijh

Feature

Therefore, introduce a property wrapper called Distinct.

Property wrapped in Distinct can be obtained through state method and keyPath. At this time, it is executed only when the value changes, and the above-mentioned problems can be solved. This method simplifies state subscriptions and is more intuitive.

// before
reactor.state.map { $0.value }.distinctUntilChanged()
// after
reactor.state(\.$value) 

In order to apply Distinct property wrappers, the type of property must be hashable.
Here's an improved code.

class TestReactor: Reactor {
  
  struct State {
    @Distinct var items: [String] = ["a", "b", "c"]
  }
}
class TestViewController: UIViewController, View {

  func bind(reactor: TestReactor) {
    reactor.state(\.$items)
      .subscribe(onNext: { items in
        // βœ… Except for the first run, it only works when the value changes.
      })
      .disposed(by: disposeBag)
  }
}

@SamarthKejriwal
Copy link

Hi @haeseoklee, can you please explain what exactly would be the difference between @Pulse and the new property wrapper @Distinct that you are introducing?

@haeseoklee
Copy link
Author

@SamarthKejriwal

Pulse responds whenever a value is injected, but Distinct responds whenever a value changes.
Let me show you a simple example.

Pulse

class TestReactor: Reactor {
  enum Action {
    case showAlert(message: String?)
  }

  enum Mutation {
    case setAlertMessage(String?)
  }
  
  struct State {
    @Pulse var alertMessage: String?
  }
  
   let initialState = State()

  func mutate(action: Action) -> Observable<Mutation> {
    switch action {
    case let .showAlert(message):
      return Observable.just(Mutation.setAlertMessage(message))
    }
  }

  func reduce(state: State, mutation: Mutation) -> State {
    var newState = state
    switch mutation {
    case let .setAlertMessage(alertMessage):
      newState.alertMessage = alertMessage
    }
    return newState
  }
}
var receivedAlertMessages: [String] = []

reactor.pulse(\.$alertMessage)
      .compactMap { $0 }
      .subscribe(onNext: { alertMessage in
        receivedAlertMessages.append(alertMessage)
      })
      .disposed(by: disposeBag)
      
reactor.action.onNext(.showAlert(message: "3"))
reactor.action.onNext(.showAlert(message: "3"))
reactor.action.onNext(.showAlert(message: "3"))
reactor.action.onNext(.showAlert(message: "3"))
reactor.action.onNext(.showAlert(message: "3"))

print(receivedAlertMessages) // ["3", "3", "3", "3", "3"]

Distinct

class TestReactor: Reactor {
  enum Action {
    case showAlert(message: String?)
  }

  enum Mutation {
    case setAlertMessage(String?)
  }
  
  struct State {
    @Distinct var alertMessage: String?
  }
  
   let initialState = State()

  func mutate(action: Action) -> Observable<Mutation> {
    switch action {
    case let .showAlert(message):
      return Observable.just(Mutation.setAlertMessage(message))
    }
  }

  func reduce(state: State, mutation: Mutation) -> State {
    var newState = state
    switch mutation {
    case let .setAlertMessage(alertMessage):
      newState.alertMessage = alertMessage
    }
    return newState
  }
}
var receivedAlertMessages: [String] = []

reactor.state(\.$alertMessage)
      .compactMap { $0 }
      .subscribe(onNext: { alertMessage in
        receivedAlertMessages.append(alertMessage)
      })
      .disposed(by: disposeBag)
      
reactor.action.onNext(.showAlert(message: "3"))
reactor.action.onNext(.showAlert(message: "3"))
reactor.action.onNext(.showAlert(message: "3"))
reactor.action.onNext(.showAlert(message: "3"))
reactor.action.onNext(.showAlert(message: "3"))

print(receivedAlertMessages) // ["3"]

@leedh2004
Copy link

leedh2004 commented Mar 24, 2022

It's certainly a good attempt, but in this case, I think the existing method of adding distinctUntilteChanged() to the state property is more obvious than using @Distinct. πŸ€”

@haeseoklee
Copy link
Author

Thanks for your comment! @leedh2004
If you think it's not obvious, can you share why? So what do you think about changing the naming to @Unique or @DistinctUntilChanged ?

@leedh2004
Copy link

leedh2004 commented Mar 24, 2022

I thought that most people were more familiar with the traditional way, and traditional way(adding distinctUntilChanged() to state) isn't annoying work for me.
It's just my personal opinion, so you can ignore it. πŸ˜…

@OhKanghoon
Copy link
Member

OhKanghoon commented Mar 25, 2022

Thank you for your new proposal!

reactor.state.map(\.foo)
  . distinctUntilChanged()

I think most people are more familiar with the existing way.

reactor.state.map(\.foo)
reactor.state(\.foo)

reactor.state.map(\.foo)
  . distinctUntilChanged()
reactor.state(\.foo)
  . distinctUntilChanged()

When a new state function is created, the existing state has the same name.
This can make most people uncomfortable using autocomplete, so I'm careful to approve it.

@haeseoklee
Copy link
Author

haeseoklee commented Mar 25, 2022

@OhKanghoon
Thank you so much for your feedback.
My intention was to reduce repetitive code and was to enforce the same effect as distintUntilChanged

However, if you feel unfamiliar with this method and have the potential to confuse existing code, I think it may do more harm than good. I don't know if it could be improved for the better (renaming or something...), but as you said, I'm cautious and worried that people will have to learn new features.

In that regard, I will take time to reconsider my proposal. :)
Thanks again for the great feedback!!

@haeseoklee haeseoklee closed this Mar 26, 2022
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

Successfully merging this pull request may close these issues.

None yet

4 participants