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

Added new method to StateContainer that takes a StateSequence instace #48

Merged
merged 5 commits into from
Jun 16, 2024

Conversation

bdunay3
Copy link
Contributor

@bdunay3 bdunay3 commented Jun 14, 2024

Description

I've been using VSM more and more inside of Swift Concurrency code and notice that I keep repeating the same mistake over and over. When ever I write a function on a state model that returns a StateSequence, I mark that function as async even though inside said function I don't call any async functions. I believe it's because current the only way to have StateContainer to observe a StateSequence you need to call the observeAsync method which takes an async closure that returns a StateSequence.

Take this method as an example;

func getFirstPage(using datasource: Datasource) async -> StateSequence<PagedListViewState> {
    .init({ .fetchingFirstPage }, { await fetchingPage(using: datasource) })
}

I wrote it this way because when I call it I typically write it like this:

$state.observeAsync { await viewModel.getFirstPage(using: datasource) }

But this is entirely unnecessary because creating a new instance of StateSequence is not an async options. StateSequence models an array of async closures that return a State. iterating through the sequence is an async operation.

So in this PR I added a new overload to the observe method that I think is a better fit when you need to only observer a StateSequence, but creating that sequence is not asynchronous. Taking the same code as above I would now write the function that returns the StateSequence like this:

func getFirstPage(using datasource: Datasource) -> StateSequence<PagedListViewState> {
    .init({ .fetchingFirstPage }, { await fetchingPage(using: datasource) })
}

And when I go to observe those state changes I would call the new observe method that would look like this:

$state.observe(viewModel.getFirstPage(using: datasource))

I think this should prevent other users from making the same mistake I keep making.

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change
  • Refactor
  • Documentation
  • Other (please describe)

Checklist

  • I have read the contributing guidelines
  • Existing issues have been referenced (where applicable)
  • I have verified this change is not present in other open pull requests
  • Functionality is documented
  • All code style checks pass
  • New code contribution is covered by automated tests
  • All new and existing tests pass

…e. This is different from the method that takes an async closure that returns a StateSequence.
Sources/VSM/StateContainer/StateObserving.swift Outdated Show resolved Hide resolved
Sources/VSM/StateContainer/StateObserving.swift Outdated Show resolved Hide resolved
Sources/VSM/StateContainer/StateObserving.swift Outdated Show resolved Hide resolved
Sources/VSM/StateContainer/StateObserving.swift Outdated Show resolved Hide resolved
@albertbori albertbori merged commit 1a83386 into main Jun 16, 2024
3 checks passed
@albertbori albertbori deleted the observe_statesequence_overload branch June 16, 2024 21:57
@albertbori
Copy link
Collaborator

Fantastic addition, Bill! Thanks!

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.

2 participants