Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Name collisions in UIImageView category #350

Closed
matej opened this issue Mar 27, 2013 · 9 comments · Fixed by #770
Closed

Name collisions in UIImageView category #350

matej opened this issue Mar 27, 2013 · 9 comments · Fixed by #770
Milestone

Comments

@matej
Copy link
Contributor

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.

@rs
Copy link
Member

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.

@Vyazovoy
Copy link
Contributor

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 :)

@0xced
Copy link
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.

@PaulWoodIII
Copy link

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.

@sibljon
Copy link
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

@kastiglione
Copy link

👍

@kastiglione
Copy link

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).

@bpoplauschi
Copy link
Member

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
Copy link
Member

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
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants