Make the api more Swift-y

[gkaimakas]

Currently AlamofireImage provides a set of extensions for

• UIButton
• UIImage
• UIImageView

The functions that are provided are all prefixed with af_, e.g: af_setImage making for an un-swifty api.

It would make sense to move this functionality to a struct that is returned from UIButton, UIImage and UIImageView. The api the could become something like that:

let imageView = UIImageView()
imageView.af.setImage(...)

There is a very nice discussion on ReactiveCocoa about the matter and an example can be found at RxSwift.

[jshier]

One major issue I see with this is that it makes the extension API far less discoverable. Right now, if I just start typing "set" Xcode will fuzzily autocomplete the AFI extensions for us. Under this model, users would have to know that all of that functionality is hidden under an af property. It is, however, an interesting pattern. @cnoon?

[cnoon]

I think it's a cool idea, but it certainly complicates things...by a lot. Here's a couple of examples on how it could "possibly" work on the UIImage extension:

Option 1

Create custom StaticExtender and Extender types that are exposed as implemented properties on a UIImage extension.

public class UIImageStaticExtender {
    init() {}

    public func threadSafeImage(with data: Data) -> UIImage? {
        lock.lock()
        let image = UIImage(data: data)
        lock.unlock()

        return image
    }

    public func threadSafeImage(with data: Data, scale: CGFloat) -> UIImage? {
        lock.lock()
        let image = UIImage(data: data, scale: scale)
        lock.unlock()

        return image
    }
}

public class UIImageExtender {
    let image: UIImage

    init(image: UIImage) {
        self.image = image
    }

    public var inflated: Bool {
        get {
            if let inflated = objc_getAssociatedObject(image, &UIImage.AssociatedKey.inflated) as? Bool {
                return inflated
            } else {
                return false
            }
        }
        set {
            objc_setAssociatedObject(image, &UIImage.AssociatedKey.inflated, newValue, .OBJC_ASSOCIATION_RETAIN_NONATOMIC)
        }
    }

    public func inflate() {
        guard !inflated else { return }

        inflated = true
        _ = image.cgImage?.dataProvider?.data
    }
}

extension UIImage {
    public static var af: UIImageStaticExtender { return UIImageStaticExtender() }
    public var af: UIImageExtender { return UIImageExtender(image: self) }
}

Option 2

Go a step further and implement StaticExtendable and Extendable protocols and use extensions to automatically create generic StaticExtender and Extender instances. Then declare UIImage to conform to StaticExtendable and Extendable.

public class TypeExtender<OwnerType: AnyObject> {
    init() {}
}

public class Extender<Owner: AnyObject> {
    let owner: Owner

    init(owner: Owner) {
        self.owner = owner
    }
}

public protocol TypeExtendable: class {}
public protocol Extendable: class {}

extension TypeExtendable {
    public static var af: TypeExtender<Self> { return TypeExtender<Self>() }
}

extension Extendable {
    public var af: Extender<Self> { return Extender<Self>(owner: self) }
}

extension TypeExtender where OwnerType: UIImage {
    public func threadSafeImage(with data: Data) -> UIImage? {
        lock.lock()
        let image = UIImage(data: data)
        lock.unlock()

        return image
    }

    public func threadSafeImage(with data: Data, scale: CGFloat) -> UIImage? {
        lock.lock()
        let image = UIImage(data: data, scale: scale)
        lock.unlock()

        return image
    }
}

extension Extender where Owner: UIImage {
    public var inflated: Bool {
        get {
            if let inflated = objc_getAssociatedObject(owner, &UIImage.AssociatedKey.inflated) as? Bool {
                return inflated
            } else {
                return false
            }
        }
        set {
            objc_setAssociatedObject(owner, &UIImage.AssociatedKey.inflated, newValue, .OBJC_ASSOCIATION_RETAIN_NONATOMIC)
        }
    }

    public func inflate() {
        guard !inflated else { return }

        inflated = true
        _ = owner.cgImage?.dataProvider?.data
    }
}

extension UIImage: TypeExtendable, Extendable {}

Discussion

First off, both of these greatly complicate the implementation. I want to state the obvious and ask whether the _ vs. the . is really worth this overhead. IMO, it's questionable.

Discoverability

Now as for discoverability, I totally see your point @jshier. I personally think switching to an af. prefix would help for discoverability for me. If I knew I was looking to use AFI extensions, typing image.af. would be pretty nice because it would only show me the actual AFI extensions rather than everything. The kicker is that I'd have to remember the af property.

I'm not sure where everyone in the community would land there.

I played around with this with the implementations above and it works great. UIImage.af. will definitely give you autocomplete for the threadSafeImage and image.af. will give you the inflated APIs. Pretty slick.

Type and Object Extensions

Unfortunately, we have both static and instance extensions on most of the types in AFI which requires us to create two different protocols if we want to be consistent. We should have API extensions directly on the type, then require you to use af. on the instance. They should be match.

image.af.inflate()
let safeImage = UIImage.af.threadSafeImage(with: Data())

This is pretty frustrating but I can't see a better way to do it. Open to suggestions here...

Backwards Compatibility

Now another important thing to note here is how we would maintain backwards compatibility if we decided to do this. We could add the new APIs and deprecate the old if we wanted, but we'd need to keep them around for the lifetime of AFI 3.x if we were going to perfectly follow semver.

I think it would be pretty weird for people if we had both the af. and af_ versions in the same release. It certainly would create some confusion. We could consider just deleting the af_ variants and push a minor release, but that would certainly aggravate some people.

We could also bump to AFI 4.x, but I'd rather avoid that if possible.

Property Names

Something missing from all the discussions that I've read are the fact that the properties could collide with another library. If all the OSS libraries start using binding, then we're in trouble. If two frameworks write public extensions on UIImage adding a binding property, it will cause ambiguity issues in an app or framework that consumes both frameworks containing the binding extension.

I've seen people arguing that this isn't what would happen because of modules and Swift. Obviously they haven't actually tried it yet. Swift module namespacing only works for the types declared in that framework. As soon as other frameworks start writing extensions on those types, naming collisions are totally a thing and prefixing (or some other approach) is certainly required.

Summary

Overall I'm both excited and annoyed with the opportunity here. I really don't want to add all this extra overhead, but I definitely want to conform to the Swift 3 conventions as much as possible. Coupled with the backwards compatibility, I'm not really sure whether we want to wait on this or dive right in. I'm also concerned about deprecating the af_ variants as well as having both available in the 3.x versions.

Thoughts everyone?

[cnoon]

Given the complexity of this type of change, we're going to punt this off to AFI 4. I've added a card to our Trello backlog. I'm still not convinced these changes are wise to make, but I do think it's worth investigating more once the time comes.

[cnoon]

Closing this out for PR #394. Please redirect all future comments there. Cheers.