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

Utilize Dependency Injection for ContactsRepository #228

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

SuhasDissa
Copy link
Member

I know you didn't expect this, but I wanted to utilize some dependency injection.

  • Moved the code from ContactHelper into a ContactRepository
  • Created a singleton instance of each repository inside the Application class
  • Created ContatsSource enum to switch between Device and Local Repositories

There's no clear performance benefit from doing this, but the overall code looks cleaner and easy to understand.
And since the repository is a singleton now, we can do some caching to improve performance in the future.

PS: I had to do all the changes in one commit because the code was too intermingled.

Whether or not you use the changes is up to you. I just wanted to give you a heads up.

Copy link
Member

@Bnyro Bnyro left a comment

Choose a reason for hiding this comment

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

Code lgtm and didn't find any issues when testing.
Well done!

@Bnyro
Copy link
Member

Bnyro commented Aug 25, 2023

(only resolved the conflicts, you can merge it if you want)

@SuhasDissa SuhasDissa merged commit 98ca873 into you-apps:main Aug 25, 2023
1 check passed
@SuhasDissa SuhasDissa deleted the di branch August 26, 2023 12:48
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