Skip to content

feat: add interactor - #1532

Draft
zafar4aev wants to merge 1 commit into
developfrom
feature/interactor-concept
Draft

feat: add interactor#1532
zafar4aev wants to merge 1 commit into
developfrom
feature/interactor-concept

Conversation

@zafar4aev

Copy link
Copy Markdown
Contributor

Description of the changes

@bigearsenal bigearsenal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In general I like this idea.


private func bindToInteractor() {
interactor.transferAccountsPublisher
.assignWeak(to: \.transferAccounts, on: self)

@bigearsenal bigearsenal Aug 2, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For iOS 15 we can use assign(to: &$transferAccounts) directly (without .store)
https://developer.apple.com/documentation/combine/publisher/assign(to:)

CryptoEthereumAccountsAggregator().transform(input: (state.value, actions))
}

private lazy var solanaAccountsPublisher = Publishers

@bigearsenal bigearsenal Aug 2, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we mark it at lazy var (should interactor always keep reference of this publisher)? I think computed property is enough, since it doesn't required to store Publisher in memory

private var subscriptions = Set<AnyCancellable>()
private var defaultsDisposables: [DefaultsDisposable] = []

private var _transferAccountsPublisher = CurrentValueSubject<[any RenderableAccount], Never>([])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This can be named transferAccountsSubject to distinguish with transferAccountPublisher.

import Web3
import Wormhole

protocol CryptoAccountsInteractorProtocol {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We agreed (when we didn't have you with us) naming protocol as CryptoAccountsInteractor and Implementation with suffix Impl (CryptoAccountsInteractorImpl). Let's follow this pattern

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