GT-2839 localization settings domain layer #2867
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #2867 +/- ##
===========================================
+ Coverage 53.49% 54.28% +0.79%
===========================================
Files 1343 1353 +10
Lines 66308 67121 +813
===========================================
+ Hits 35471 36437 +966
+ Misses 30837 30684 -153 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
levieggertcru
left a comment
levieggertcru
left a comment
There was a problem hiding this comment.
Hey @rachaelblue looking good. I left a couple notes that are up to you if you want to change. Let me know your thoughts on those notes I left.
For the strings domain model and expose that directly to the View so you don't have to perform additional mapping.
...ures/PersonalizedTools/Presentation/LocalizationSettings/LocalizationSettingsViewModel.swift
Outdated
Show resolved
Hide resolved
| import Foundation | ||
| import Combine | ||
|
|
||
| class ViewLocalizationSettingsUseCase { |
There was a problem hiding this comment.
Just a note and no change is needed here, I noticed in MPDX my use cases became smaller because it was easier to encapsulate smaller pieces of the business logic. It will all depend on how complex the use case gets.
| private let getCurrentAppLanguageUseCase: GetCurrentAppLanguageUseCase | ||
| private let getCountryListUseCase: GetLocalizationSettingsCountryListUseCase | ||
| private let searchCountriesUseCase: SearchCountriesInLocalizationSettingsCountriesListUseCase | ||
| private let viewLocalizationSettingsUseCase: ViewLocalizationSettingsUseCase |
There was a problem hiding this comment.
Also just a note, if you don't have to store the use case I'm fine with omitting it here since it will be bound to the scope of the init because of the cancellables reference.
There was a problem hiding this comment.
Doesn't the cancellable hold onto the subscriber separately though? So the use case could still get deallocated if we remove this reference, even though the subscriber is alive?
There was a problem hiding this comment.
Yes, I believe you're right. The UseCase would get deallocated, however the publisher returned from the UseCase would still be alive as long as the cancellable is alive.
I don't think it would cause any issues still since the UseCase isn't holding anything that is mutable. Let's just leave the UseCase referenced for now. I'd like to do some research a little more on this one.
There was a problem hiding this comment.
Ok interesting. Curious to know how that works!
No description provided.