Glide: How should fitCenter be implemented using 4.x API

[electrolobzik]

Glide Version: 4.0.0 SNAPSHOT from 10/22/2016

How should I implement

requestManager.load(avatarUrl)
        .fitCenter()
        .placeholder(R.drawable.profile_photo_stub)
        .into(viewHolder.avatarImg);

using 4.x API?

Looks like it should be something like

requestManager.load(avatarUrl)
        .apply(RequestOptions.fitCenterTransform(getContext()))
        .apply(RequestOptions.placeholderOf(R.drawable.profile_photo_stub))
        .into(viewHolder.avatarImg);

but it seems too heavy and complex for me.

Why fitCenterTransform requires context and doesn't get it from the requestManager?

[TWiStErRob]

I prefer this style (it shows more that an object is created, less method calls, more code completion):

glide
        .load(avatarUrl)
        .apply(new RequestOptions()
                .fitCenter(getContext())
                .placeholder(R.drawable.profile_photo_stub)
        )
        .into(viewHolder.avatarImg);

Notice that you can cache, name and reuse your RequestOptions objects, they're even injectable from outside if you want to go that far.

// In adapter ctor
RequestOptions avatarOptions = new RequestOptions()
        .fitCenter(context)
        .placeholder(R.drawable.profile_photo_stub);
// In bind
 glide
        .load(avatarUrl)
        .apply(avatarOptions)
        .into(viewHolder.avatarImg); 

I think the apply was introduced, because we can configure parts of the flow via Options now. You can create your own MyRequestOptions builder and create named methods for custom options. This means that you can essentially extend the Glide builder API with compile time methods.

If you want more info you should probably read https://docs.google.com/document/d/13jhhlAqcKU2kweY2PlKW8HMPgTxXvu3Qv8A8dPBJP0Q

As for the context param, that's a good point. It's probably a side effect of RequestOptions not knowing about RequestBuilder fields. @sjudd why do we need the context?
Mind you that you can always write your own extension of RequestOptions that has a parameterless fitCenter by adding a static Context to use or one in the constructor.

[electrolobzik]

Thank you for explanation and the link (found a lot of useful information from it). It really makes sense.

I have one more question. As I can see we still can implement builder methods like fitCenter() inside RequestManager like this:

public RequestBuilder<TranscodeType> fitCenter() {
    RequestOptions requestOptions = new RequestOptions().fitCenter(getContext());
    return apply(requestOptions);
}

This way Glide v4 will get all it flexibility and power but save compatibility with the code written with v3 API or allow users to easily move from Picasso. To force users to reuse RequestOptions we can just make this builder methods deprecated. What do you think about it?

[TWiStErRob]

Reusing RequestOptions is not required, you can use them inline as well. @sjudd what do you think about the deprecated convenience methods with JavaDoc for the new version? I agree it could help migration, because even IDEA doesn't really have a way to do this. Migration there only supports imports, and structural replace cannot be easily exported/imported as far as I found.

@electrolobzik the only thing that bugs me that those deprecated methods would have to be there indefinitely, because Picasso and others likely won't have this API. Mind you that you don't need to explicitly tell the transformation most of the time, it is being read from ImageView.scaleType.

[electrolobzik]

If you guys think that using new RequestOptions inline is normal we can remain old API methods even not deprecated. I like old API. It is simple, intuitive and handy. And I believe that most of Picasso/Glide users like it especially for it's simple and beautiful API. For some users it is enough. For those of them who want more flexibility new API will be available. I don't understand the reason of removing this API from Glide.

[TWiStErRob]

I think it's simply to prevent a lot of duplicate functionality, or because in some cases fitCenter is not possible, though you can probably always apply it anyway. Let's see what @sjudd's stance is, I'm pretty meh about it; I'll prefer and advocate the new API in issues, even if the old API is added back.

[androidTH]

Good!

[sjudd]

I'm relatively tempted to just make the standard RequestBuilder implement RequestOptions since it's an interface anyway.

The original idea was that users can subclass RequestOptions to add their own custom options and create their own fluent API, but I haven't actually seen anyone doing that.

[TWiStErRob]

@sjudd I think that needs an example/blog post to get a starting point. It may be considered too advanced for some developers. There are a lot of comments on GitHub, where some struggle with a simple custom loader/fetcher.

[sjudd]

The requested API (RequestOptions methods directly on RequestBuilder) is implemented via Glide's generated API. I'll try and write up some documentation about how that works this weekend. Leaving this open to track the documentation.

[sjudd]

http://sjudd.github.io/glide/doc/generatedapi.html