Skip to content

Make the cache provider an injectable dependency#20

Merged
pknotfound merged 5 commits intopknotfound:masterfrom
Mek101:cache_provider
Sep 16, 2022
Merged

Make the cache provider an injectable dependency#20
pknotfound merged 5 commits intopknotfound:masterfrom
Mek101:cache_provider

Conversation

@Mek101
Copy link
Copy Markdown
Contributor

@Mek101 Mek101 commented Sep 15, 2022

This removes the hard-dependency on SharedPreference as a cache implementation and allows the user to supply their own

@pknotfound
Copy link
Copy Markdown
Owner

Hello @Mek101
Thank you for your contribution

Can you please include the code where you use your injectable dependency of cache provider?

Thank you

@Mek101
Copy link
Copy Markdown
Contributor Author

Mek101 commented Sep 15, 2022

You mean in the example application?

@pknotfound
Copy link
Copy Markdown
Owner

Yes.
I just tested out your code, and it works fine though.

Your solution works alright, but has one potential issue which I see. If the user wishes to implement caching, they need to create a class on their own and pass it as a parameter, but what if a user wishes to use the default shared preferences rather than creating one on their own. Also, this solution eliminates the ability for the user to choose if they wish to implement caching or not.

What are your thoughts?

Thanks

@Mek101
Copy link
Copy Markdown
Contributor Author

Mek101 commented Sep 15, 2022

what if a user wishes to use the default shared preferences rather than creating one on their own.

They can simple create a SharedPrefsCacheProvider instance and pass it as an argument to the OpenGraphParser constructor.

this solution eliminates the ability for the user to choose if they wish to implement caching or not.

The cacheProvider parameter can be null, allowing the user to disable caching by providing no implementation.

I also added how to use the default provider with in the example application

@Mek101
Copy link
Copy Markdown
Contributor Author

Mek101 commented Sep 15, 2022

I also made the member functions of CacheProvider suspendable so they can be used in coroutine-heavy contexts.

@pknotfound
Copy link
Copy Markdown
Owner

@Mek101
This is great. I now understand how it would work. Thank you for your explanation.

I will test out your changes soon.

Thank you

@pknotfound
Copy link
Copy Markdown
Owner

@Mek101
Thank you for your contribution.

Merging your changes

@pknotfound pknotfound closed this Sep 16, 2022
@pknotfound pknotfound reopened this Sep 16, 2022
@pknotfound pknotfound merged commit d13ac57 into pknotfound:master Sep 16, 2022
@Mek101 Mek101 deleted the cache_provider branch September 16, 2022 10:50
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