Skip to content

Rename ImageLoaderProvider to ImageLoaderFactory.#311

Merged
colinrtwhite merged 2 commits into
masterfrom
colin/rename_imageloaderfactory
Mar 12, 2020
Merged

Rename ImageLoaderProvider to ImageLoaderFactory.#311
colinrtwhite merged 2 commits into
masterfrom
colin/rename_imageloaderfactory

Conversation

@colinrtwhite
Copy link
Copy Markdown
Member

Thought about this some more and Provider doesn't feel right even though it's in line with AndroidX. Rename to ImageLoaderFactory and newImageLoader to imply creation. This is continues follows the API naming convention in OkHttp. @Jawnnypoo you were right!

Copy link
Copy Markdown
Member

@Jawnnypoo Jawnnypoo left a comment

Choose a reason for hiding this comment

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

<3 OkHttp

@Synchronized
fun setImageLoader(provider: ImageLoaderProvider) {
imageLoaderProvider = provider
fun setImageLoader(factory: ImageLoaderFactory) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should the method be setImageLoaderFactory instead?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, I get it now. Probably best to keep it like this so the consumer gets a choice of how they want to provide the image loader 👍

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep, I think it'll make more sense too when we can make ImageLoaderFactory a fun interface in Kotlin 1.4 so it's callable using Coil.setImageLoader { ImageLoader(context) }. Mostly I wanted to give both methods the same name to show they overwrite each other.

@colinrtwhite colinrtwhite merged commit fd7e2aa into master Mar 12, 2020
@colinrtwhite colinrtwhite deleted the colin/rename_imageloaderfactory branch March 12, 2020 22:46
colinrtwhite added a commit that referenced this pull request Oct 5, 2022
* Rename ImageLoaderProvider to ImageLoaderFactory.

* Emphasize.
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