Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Name collisions in UIImageView category #350

Closed
matej opened this Issue Mar 27, 2013 · 9 comments

Comments

Projects
None yet
8 participants
Collaborator

matej commented Mar 27, 2013

Hi Olivier,

I've noticed that UIImageView+WebCache collides with some methods in UIImageView+AFNetworking (e.g., setImageWithURL: and setImageWithURL:).

I think it's pretty likely that those two libraries are used in the same project, at least that's true fro several of my projects, so it would make sense to do something about this and prevent undefined runtime behavior. Prefixes are the most robust solution, although simply renaming the method name to something slightly different would also probably work fine for now, without sacrificing legibility.

It would also be interesting to hear what @mattt has to say about this.

My solution for this problem is to simply avoid the colliding methods and use one of the long-form methods that differs from the ones defined in the AFNetworking category. This might me a well enough solution for me personally, but I'm pretty sure that sooner or later someone is going to run into some problems with this.

Owner

rs commented Mar 27, 2013

SDWebImage introduced those methods in 2009 and is the main purpose of the library. I agree it would have been a good idea to prefix them in the first place, but changing the name of those methods now would break backward compatibility.

Contributor

Vyazovoy commented Mar 27, 2013

I agree with @matej because it is real problem (not only in this repo) naming new variables, types, methods without unique prefix. I don't understand what do you mean when talk about backward compatibility. If you change the name of methods in new version (for example), then other programmers should change those code in 10-20 places. It is not so hard work, but it will fix the problem and all subsequent. @matej, you should copy your post to AFNetworking :)

Contributor

0xced commented Apr 12, 2013

In order not to break backward compatibility, you could mark these methods as deprecated in the interface and implement them dynamically with +[NSObject resolveInstanceMethod:] for example.

I just want to add that I ran into this problem as well and quickly made some changes on this fork: https://github.com/PaulWoodIII/SDWebImage

I'm not asking for a merge here but maybe it'll help someone else interested in this issue.

@0xced if you could share some code on how to use resolveInstanceMethod I could add that in as well.

Contributor

sibljon commented Jul 9, 2013

Hi all,

For anyone looking to use both SDWebImage and AFNetworking as-is, here's my caveman solution to discourage use of the colliding methods:

https://gist.github.com/sibljon/5957892

It's obviously not future proof, and is prone to breaking if the implementation of these methods changes in AFNetworking or SDWebImage. It's far from an ideal solution. In other words, at your own risk!

Jon

@0xced 0xced referenced this issue in AFNetworking/AFNetworking Jul 17, 2013

Closed

Prefix all methods in categories #1142

👍

In addition to the already suggested transition options, another way could be to use conditional compilation and give those of us who want prefixes the ability to turn on prefixing by defining a preprocessor macro (ex. SD_WEB_IMAGE_USE_PREFIX).

Collaborator

bpoplauschi commented Jun 11, 2014

An update to this thread:

  • the two methods that collide are setImageWithURL: and setImageWithURL:placeholderImage:
  • Mattt stated in AFNetworking #1142:

In AFNetworking 2.0, all categories on public classes (namely the UIKit extensions) will be made available as subspecs through CocoaPods. Because the decision to pull in these categories would be explicitly made by the developer, the need for prefixing is obviated.

This means that developers (like @matej, myself, ...) can specify just the AFNetworking core subspec i.e. pod 'AFNetworking/NSURLConnection' or pod 'AFNetworking/NSURLSession'

  • using prefixes for methods is not ideal, as it impacts the autocomplete from Xcode/AppCode/...
  • there is always the option @matej suggested, to use methods that are only in SDWebImage like setImageWithURL:placeholderImage:options: or setImageWithURL:completed:

Given those, I don't see a solid solution, so I would just leave things as they are.
What do you think? @rs @matej @0xced

@bpoplauschi bpoplauschi added this to the Future milestone Jun 17, 2014

@bpoplauschi bpoplauschi modified the milestones: 3.7.0, Future Jun 20, 2014

Collaborator

bpoplauschi commented Jun 20, 2014

Good news guys. This was fixed in #770, all those category methods now have a sd_ prefix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment