SDWebImage - What's your name

[bpoplauschi]

Per #2285, review all the entity names before releasing 5.0.0-beta.

• [ ] stop using SDWebImage as a default prefix for entities. SD is our official prefix. Web means the entity applies only to web resources. Image refers only to images.

Proposals:

• [x] rename SDWebImageCachesManager* to SDImageCachesManager* #2292
• [x] rename SDWebImageCache* to SDImageCache* #2292
• [x] rename NSImage (Additions) to NSImage (Compatibility) #2293
• [x] rename SDWebImageTransformer* to SDImageTransformer* #2304
• [x] rename SDWebImageCacheKeyFilter to SDCacheKeyFilter
• [x] rename SDWebImageProgressiveDownload to SDWebImageProgressiveLoad #2294
• [x] rename SDWebImageFrame to SDImageFrame #2305
• [x] rename SDWebImageCoderHelper to SDImageCoderHelper #2305
• [x] rename SDWebImage*Coder to SDImage*Coder #2306
  
  
  
  • SDWebImageCoder protocol -> SDImageCoder
  
  
  • SDWebImageProgressiveCoder protocol -> SDProgressiveImageCoder
  
  
  • SDWebImageAnimatedCoder protocol -> SDAnimatedImageCoder
  
  
  • SDWebImageCodersManager -> SDImageCodersManager
  
  
  • SDWebImageImageIOCoder -> SDImageIOCoder
  
  
  • SDWebImageGIFCoder -> SDImageGIFCoder
  
  
  • SDWebImageWebPCoder -> SDImageWebPCoder
  
  
  • SDWebImageAPNGCoder -> SDImageAPNGCoder
  
  
• [x] rename SDWebImageLoader* to SDImageLoader*

Top level API - to be discussed if we make those

• [ ] rename SDWebImageManager* to SDImageManager*
• [ ] rename SDWebImageDownloader* to SDImageDownloader*

CC @dreampiggy @skyline75489 @mythodeia

[dreampiggy]

Maybe some image decoding/encoding related class can also be renmaed. But I wonder does this changes's gain is much more than the gain ?

Anyway, here is the suggestion I think can be renamed without that SDWebImage prefix to simple SDImage prefix.

Recommend to change (Not widely used by user in 4.x)

• SDWebImageCoderHelper Class -> SDImageCoderHelper (Actualy this class is really independent, only include SDWebImageCompat.h, even without that SDWebImageDefine.h)
• SDWebImageFrame Class -> SDImageFrame (Really independent too)

Carefuly considered to change (May be widely used by user in 4.x), it's recommend to keep current naming.

• SDWebImageCoder Protocol -> SDImageCoder
• SDWebImageProgressiveCoder Protocol -> SDProgressiveImageCoder
• SDWebImageAnimatedCoder Protocol -> SDAnimatedImageCoder

• SDWebImageCodersManagerClass -> SDImageCodersManager

• SDWebImageImageIOCoder Class -> SDImageIOImageCoder (Not recommend, SDWebImage here means the Image/IO coder is provided by SDWebImage framework. Like the meaning it's maintained by SDWebImage's maintainer)

• SDWebImageGIFCoder Class -> SDGIFImageCoder (Same as above)
• SDWebImageWebPCoder Class -> SDWebPImageCoder (Same as above)
• SDWebImageAPNGCoder Class -> SDAPNGImageCoder (Same as above)

[dreampiggy]

I'm not recommend to change SDWebImageManager and many classes in the list above. This class are all the really common Core API. Change this will cause 4.x user really hard to migration. And the naming it's good with Web.

Our framework is create aim to Asynchronous image downloader with cache support as a UIImageView category. And SDWebImage it's the framework's naming. It's no need to change unless the naming cause misunderstanding.

I suggest to change something not really in common usage, and it's pure with non-network, non-photos or non-anything about Loading. To use that SD/SDImage without SDWebImage prefix.

For exmaple, SDWebImageTransformer is about Loading, because it's used by manager to transform downloaded image into another format. And cache also using this. So it's not so Pure.

Only something like SDWebImageCoderHelper, which does not contains any business about Loading, and it's about image, so can be renamed to SDImageCoderHelper

[bpoplauschi]

@dreampiggy I agree about people using SDWebImageManager along with the UIView extensions. But for 90% of our users, that should be it.
A 5.x major release allows us to change the interface and optimize it.

• one reason for those renaming is pure readability: shorter entity names are easier to read and the code does not extend that much
• another reason: we have the SD prefix for all the entities. That is sufficient to avoid name collisions. Now all the other parts of the name need to make sense:
  
  
  
  • SDWebImageManager is a manager that handles loading of images, not necessarily from the web. That is why the web word is confusing in it's name.
  
  
  • SDWebImageTransformer is an image transformer, its API just allows changing from an input image to an output image, there is no relation to the web
  
  
  • an even more interesting example: SDWebImageCacheKeyFilter has nothing to do with web or even image, it's just a cache key filter
  
  
  • same principle for the context names, we just don't need to carry over that redundant WebImage
  
  

[dreampiggy]

Some of changes actually are unnecessary, and more like a Philosophical problem.
The SDWebImage prefix not always means This is a class about Web & Image, come on. Some of class or headers is about This is the file using in SDWebImage framework, and represent the common thing used in framework. For example, SDWebImageCompat, SDWebImageError, SDWebImageOptions.

And, SDWebImageContext is created aim to hold anything that SDWebImageOptions can not hold. This is because of lack of Objective-C enum which can not bind Object. It does not means that context is for Web only. This two is about the options & context works hole through our Core API. And this is why they are always combined together. And we can not store all things into that context dictioinary because this API is't not easy to use (bitmask is easier)

[bpoplauschi]

This was the logic we used for naming, but it's not ideal.
It's ok that we don't agree :) so we can have a constructive discussion and decide.

My 2 suggestions:

• let's look at other libraries as example. For instance AFNetworking which for me is a model for all Objective-C open source projects. They do not name their entities as AFNetworking...., but just use the AF prefix and then use the most relevant name for that component. Examples: AFHTTPSessionManager, AFSecurityPolicy, AFURLRequestSerialization
• extend the conversation to our colleagues @mythodeia @skyline75489 @rs what do you guys think?

[mythodeia]

@bpoplauschi i am too a fan of AFNetworking and indeed its a model open source project for all others. I agree with using the SD prefix instead of SDWebImage.
I will say it again. Naming is hard and trying to find names that "work" well for both Swift and Obj-c will result in a lengthy discussion.

[dreampiggy]

It seems that there are no pending feature PR about 5.x ? So maybe it's time to talk about this naming. And maybe we can soon release 5.0.0 beta 🙂

I recentlly write a keynote about the architecture of SDWebImage. So I'm now a little agree with the naming issue.

But to say, I'm not agreed to renaming all classes in our framework, which cause unlimited changes. I'will renaming some class introduced from 5.x new feature.

I'd suggest to renaming these class, which will cause some API break, but not so much. Most of this is about image process/encoding/decoding so it's better to remove web suffix. These are all the components in the architecture of 5.x which should related to Base Module (See below)

• SDWebImageTransformer -> SDImageTransformer. This one contains a cacheKey arg, but not related to URL, so it's adoptable to rename it.
• SDWebImageFrame -> SDImageFrame
• SDWebImageCoderHelper -> SDImageCoderHelper
• SDWebImageCoder protocol -> SDImageCoder
• SDWebImageProgressiveCoder protocol -> SDProgressiveImageCoder
• SDWebImageAnimatedCoder protocol -> SDAnimatedImageCoder
• SDWebImageCodersManager -> SDImageCodersManager
• SDWebImageImageIOCoder -> SDImageIOCoder
• SDWebImageGIFCoder -> SDImageGIFCoder
• SDWebImageWebPCoder -> SDImageWebPCoder
• SDWebImageAPNGCoder -> SDImageAPNGCoder

However, I'm really not recommend to rename SDWebImageManager, or SDWebImageDownloader, which will cause huge API break change for most of user since this is Top-level API. And it more or less related to the Web because the basic function of these class is to download image from network. Photos framework support is 5.x new feature (Even it not in the main repo of SDWebImage) and I guess the user who need this feature is nearlly less than 1% 😅

The reason also available for some other class. If the class contains any information about SDWebImageManager, NSURL, which should not be renamed. Like these:

• SDWebImageCacheKeyFilter: Contains NSURL. And it's about image cache's cache key, so the WebImageCache prefix is not so unbearable..

[dreampiggy]

[bpoplauschi]

@dreampiggy yeap, we are closing in to the beta release. Just a few things we really need to do.

About naming, I think we can find some middle ground:

• go ahead with the strong renaming I proposed above
• to avoid breaking top level API, we can just keep some aliases around

@compatibility_alias SDWebImageManager SDImageManager;

Note: using typedef to create an alias will result in a compiler error

typedef SDWebImageManager SDImageManager;

SDImageManager.h:240:27: Redefinition of 'SDImageManager' as different kind of symbol

[bpoplauschi]

About my idea above, the one downside is imports file names will also changes, so for those who have explicitly #import <SDWebImage/SDWebImageManager.h> it will break. Not sure if many did this.

[dreampiggy]

@bpoplauschi That compatibility_alias is not magic solution. Especially when you use something like type check, @class, @protocol, isKindOfClass:. They all fail because there are only one SDImageManager class in the runtime. It just a typedef. This will cause many potential problems.

I think, that manager is designed as a bridge to cache & downloader. In 5.x we support custom loader protocol. But actually the usage for that loader protocol is about Custom Network , Third-party Cloud service. For example, our company using Cronet instead of NSURLSession for most of network request. Which build on the top of Socket API and supoprt cross-platform usage. Though we have custom download operation, but that SDWebImageDownloadOperation is hard to modify because it tied to NSURLSession stack.

I guess maybe for most user, that SDWebImageLoader protocol is just SDWebImageDownloader protocol. So keeping the Web make sense...After I create that PR, I realize that we can adopt the #1970 feature into that protocol. But this is not the main propose of that custom loader protocol created for...

[bpoplauschi]

@dreampiggy I appreciate you creating the PR's for those renames. They are all merged.
Let's keep going with the conversation:

• about SDWebImageManager and SDWebImageDownloader, I agree they should not be changed. One last attempt from me :)
  
  
  
  • @compatibility_alias indeed will only make the entity exist before compilation. It's very similar to a macro definition (which we use for UIImage and NSImage). The user works with the entity he wants and at compilation, that is replaced with the one we want. I don't see any issues.
  
  
  • we can also do a
  
  

#ifndef SDWebImageManager
    #define SDWebImageManager SDImageManager
#endif

• I think we can/should rename SDWebImageLoader to SDImageLoader - it was added in the 5.x branch, so no impact on the users.
• SDWebImageCacheKeyFilter -> SDCacheKeyFilter
• SDWebImageCacheSerializer -> SDImageCacheSerializer
• SDWebImageError -> SDError
• SDWebImageTransition -> SDImageTransition
• SDWebImageIndicator -> SDImageIndicator

[dreampiggy]

SDWebImageCacheSerializer, SDWebImageCacheKeyFilter are all created only for SDWebImageManager. It's just a wrapper of the original block in manager property, to solve Swift API issue described in #2261. You can check that to know why I have to use this wrapped object instead a simple block.

So I think, since this is tied with manager. We should keep these 3 have the same naming, that SDCacheKeyFilter does not make any sense. The function of this is

provide a mapping between NSURL to download and CacheKey to store to cache. So it's all about WebCache and URL loading.

So I suggest this class should contains Web prefix. But not just SD

[dreampiggy]

SDWebImageContext is introduced by #2189 . It's a enhancement for SDWebImageOptions to store any object that enum can not hold. It behaves like the arrow in that architecture picture above. Which passed from top-level API, to the bottom base module. So SDWebImageOptions & SDWebImageContext should contains the same naming.

These two are both about the SDWebImage framework level configuration, they are not a single class related to image or not. (Contains many config). So I suggest to keep SDWebImage prefix to specify that.

[bpoplauschi]

What about the other?
SDWebImageLoader, SDWebImageError, SDWebImageTransition

[rs]

Won't those name changes make it harder for users to upgrade without a clear benefit?

[bpoplauschi]

@rs I don't think there's a clear answer. There are benefits to shortening the entity names and making them more clear (or not, even that is up to debate) and a major release is the place to do so. On the other hand, I get why people might be avoiding the upgrade if it's too much work.
But for the changes we have currently applied, most of them are only 5.x features, so no friction there.
We have agreed not to changed SDWebImageManager or SDWebImageDownloader.

[dreampiggy]

@bpoplauschi Well...Another consideration from me about this renaming.

I have a idea in my mind which I want to see in the future version of SD. See the architecture picture again. Since we now seperate the top-level API between the base module. And base module only depend on system framework such as Image/IO, Core Image. And it does not import any header other than SDWebImageCompat.h. Maybe we can do this further?

Like YYImage and YYWebImage, what about put all the image processing/decoding/encoding code, into another framework (CocoaPods Pods) called SDImage. We can move all the current files like SDImageCoder, SDAnimatedImage/SDAnimatedImageView, Image Categories to that repo. Then the SDWebImage framework depend on that repo.

So the naming issue is need to re-considerate. After that SD may represent multiple repos, so that SDWebImageOptions, SDWebImageContext, SDWebImageError is valid enough because SDImage may also contains their error, options. Naming it without SDWebImage prefix will not make it clear on the contrary... :)

This can be done even in 5.x because current there are no deep dependency hell between top-level and base module. But I wonder if this one really useful or should we do this in 5.x or future...

[dreampiggy]

@bpoplauschi I can create a demo repo represent this. I think seperate the image part to another framework is easy, and can clear the dependency for our framework (No longer need WebP subspec or something related to image foramt). And make it useful for user who may only need Image part but not Web/Cache part.

This can also end up the naming discussion... Because if we adopt this architecture change to let SDWebImage depend on SDImage, SD will not only represent Web/Cache. So we may need SDWebImage prefix to indicate that this class is only availble for Web/Cache.

See SDImage's Repo

[dreampiggy]

😅 Any idea about this now ? @mythodeia @skyline75489

And I wonder if this suitable to move all the image related code into another repo SDImage ? Make SDWebImage itself depend on that repo ? It's this suitable ? What do you think ( Althrough maybe you're not develop iOS platform application. But this seems a common problem about software architecture) @rs

My personal future imagination of SDWebImage can represent like this. And this is the goal I want to see for our framework.

Out-of-box functionality and fully extensibility.

[rs]

I'm not sure to understand the need for multiple repos. Can't you have all the components in the same one? Maintaining multiple repos for libraries that depend on each other can be tedious. Any feature that will span on multiple components will require some gymnastic with multiple commits and dependency conditions.

[bpoplauschi]

I agree with @rs on this one. From my pov, all the core functionality from SDWebImage should remain into this project (easier to maintain and easier for the existing users). What I suggested above was to make sure we don't have pieces that do something else than our core lib. For instance, the concrete implementations of the transformers. Or pieces like the Bridges you mentioned in the diagram (that link SDWebImage to other modules). But for now, I don't see any obvious ones that could be made a separate project. Another example are the coders (like APNG or ProgressiveJPEG coders that are not used by a lot of our users and are well definable outside our lib.

[dreampiggy]

@bpoplauschi OK...Just a think of this approach. I guess someone may like to use that SDAnimatedImage or custom coder as a module without need of Web-Cache part in their own lib. But since there are no issue talk about this feature...Maybe we can just keep SDWebImage single framework.

[dreampiggy]

@Baichenghui @mythodeia This discussion seems take so long time. I'd like to give up 😅 If someone still think naming is important, well...I can just create a PR. After #2326 merged. All the top checkbox list are finished.

Is there anything else important to do before we release 5.0.0-beta ? I guess seems we take so long time during 5.x development...If there are no any huge pending issue (Small fix or API break can be adopted between 5.0.0-beta to the final 5.0.0-release), I think maybe we can start to release the beta version.

And, for that current FLAnimatedImage bridge, I create a repo SDWebImageFLPlugin and move the code. After the final 5.0 release, we can remove the code in the main repo. All the further issue-pr transfered to there.

[bpoplauschi]

Ok. Let's close the renaming here. I just want to release a 5.0.0-beta asap.