Document findings for .ip. functionality

[vinnyt]

@andrewdolce could you add the findings here.

[andrewdolce]

Quick Summary

The .ip. extension pattern modeled after RxSwift does not work well for SwiftWisdom because:

1. Extending the Intrepid proxy for non-class or non-protocol types appears to be pretty difficult. RxSwift only uses this pattern on classes. They have no need to extend things like String.
2. The .ip proxy cannot easily implement mutating functions on the base object if that base is a value-type.
3. Not all APIs make sense when transferred to the proxy style. Subscripts are a good example of this.

We might be able to work around these problems, but the extra complexity is probably not worth it. For more detail, read on.

Motivation behind the original idea

RxSwift uses a "proxy" pattern for organizing Rx-provided extensions on existing classes. The result is that the API looks like this:

textField.rx.textInput

Rather than something like:

textField.rx_textInput

Instead of the extension methods and properties being added onto the UITextField class itself, they are added onto the "proxy" object, which is a property on the text field. The main motivation for using this pattern in Rx seems to be mostly aesthetic, as per the original github issue:

https://github.com/ReactiveX/RxSwift/issues/826

How Rx implements this

The Reactive proxy

In RxSwift, the proxy object is an instance of the Reactive struct, defined like so:

public struct Reactive<Base> {
    /// Base object to extend.
    public let base: Base

    /// Creates extensions with base object.
    ///
    /// - parameter base: Base object.
    public init(_ base: Base) {
        self.base = base
    }
}

Note that Reactive is a generic, defined with generic type Base. The struct itself is pretty minimal. It just provides a single base property and a constructor that sets that property.

Now in considering any generic, it is always useful to ask "what does this generic do?". The purpose of a generic is to capture some common logic that works the same way regardless of the specific types involved. In other words, beyond maybe conforming to some protocol (e.g. Equatable or Hashable) we don't need to know what our generic types are.

The generic Reactive doesn't really add much logic, but I think it can be defined as "Something that has a property of type Base, and provides extensions on that Base".

Adding a Reactive proxy to each object

Consider the example call:

textField.rx.textInput

As we can see, UITextField has been extended such that each instance has a .rx property. That property is a Reactive<UITextField>, which in turns has a .base property that is a reference back to the text field. So in short:

textField.rx.base == textField

RxSwift injects this property like so:

/// A type that has reactive extensions.
public protocol ReactiveCompatible {
    /// Extended type
    associatedtype CompatibleType

    /// Reactive extensions.
    static var rx: Reactive<CompatibleType>.Type { get set }

    /// Reactive extensions.
    var rx: Reactive<CompatibleType> { get set }
}

extension ReactiveCompatible {
    /// Reactive extensions.
    public static var rx: Reactive<Self>.Type {
        get {
            return Reactive<Self>.self
        }
        set {
            // this enables using Reactive to "mutate" base type
        }
    }

    /// Reactive extensions.
    public var rx: Reactive<Self> {
        get {
            return Reactive(self)
        }
        set {
            // this enables using Reactive to "mutate" base object
        }
    }
}

import Foundation

/// Extend NSObject with `rx` proxy.
extension NSObject: ReactiveCompatible { }

There's a lot going on here so let's break it down:

First we have a protocol called ReactiveCompatible. A ReactiveCompatible object has a .rx property and a static .rx property, both of which are Reactive objects defined on the compatible's associated type.

Next we extend ReactiveCompatible to add some default getters and setters for these two properties. In each case we return a Reactive created with our own type. (TODO: I don't fully understand the empty setter implementation. It may be worth digging into.)

Finally they extend NSObject and declare that it conforms to ReactiveCompatible, thereby injecting those default properties onto every class that inherits from NSObject.

Defining extension methods for a specific type

The last step is actually using Reactive as a way to add extension methods. This is done like so:

extension Reactive where Base: UITextField {
    /// Reactive text input.
    public var textInput: TextInput<Base> {
        return TextInput(base: base, text: self.text)
    }
}

Here we add a textInput property onto any Reactive where the base type inherits from UITextField. So while Reactive by itself does very little, we can add things to it subject to conditions on the base type.

Why this doesn't work for SwiftWisdom

So far in trying to implement this syntax in wisdom, we've defined an Intrepid struct that will play the role of Reactive. We attempt to apply it to NSObject subclasses in the same way, by defining an IntrepidCompatible protocol with a .ip property.

However we run into some issues.

This doesn't seem to work for structs or enums

Working from inside the Reactive codebase, suppose I define the following four types:

class FooClass: ReactiveCompatible {}
protocol FooProtocol: ReactiveCompatible {}
struct FooStruct: ReactiveCompatible {}
enum FooEnum: ReactiveCompatible {}

Then I attempt to add some Reactive extensions like so:

extension Reactive where Base: FooClass {}
extension Reactive where Base: FooProtocol {}
extension Reactive where Base: FooStruct {}
extension Reactive where Base: FooEnum {}

As of Xcode 8.1, the last two lines cause a compiler error:

<unknown>:0: error: type 'Base' constrained to non-protocol type 'FooEnum'
<unknown>:0: error: type 'Base' constrained to non-protocol type 'FooStruct'

This seems to be because the : syntax means that Base inherits from or conforms to the given type, neither of which is possible with structs and enums. We can try revising to something like:

extension Reactive where Base == FooStruct {}
extension Reactive where Base == FooEnum {}

But this causes a different error:

Same-type requirement makes generic parameter 'Base' non-generic

It seems that the == operator in this case is meant to compare two generic types. In other words, for a generic with types U and T, it's legal for a where clause to check that U == T, but not legal for it to check against a concrete type like FooStruct or FooEnum.

So here we come to obstacle number 1:

Extending the Intrepid proxy for non-class or non-protocol types appears to be pretty difficult.

Looking through the RxSwift codebase, it looks like all of their extensions are defined on classes. They do not, as far as I can see, have things like extensions on String (a struct), which is something that Swift Wisdom needs to support.

This pattern does not work well with value types

Let's put aside for a minute the generics issue from above. After all, the end goal here is to have a syntax like someObject.ip.someFunction(). Defining these extensions on a generic Intrepid<Base> struct is nice, but maybe not a hard requirement. Instead we could do something like this:

public struct IntrepidStringExtension {
    typealias Base = String

    internal var base: Base

    init(base: Base) {
        self.base = base
    }

    // Add some extension functions here
    // ...    
}

extension String {
    public var ip: IntrepidStringExtension {
        get { return IntrepidStringExtension(base: self) }
        set { }
    }

    public static var ip: IntrepidStringExtension.Type {
        get { return IntrepidStringExtension.self }
        set { }
    }
}

So in this case we have a concretely defined IntrepidStringExtension struct that only extends strings. We define functions on the struct directly. We also could have extensions on IntrepidStringExtension, since we are not doing the Base-type comparison. Then we add the .ip object onto String with an extension, without the magic of IntrepidCompatible.

This will compile and work fine except for a pretty big caveat:

The .ip proxy cannot easily implement mutating functions on the base object if that base is a value-type.

Consider the following function:

public mutating func dropFirst() {
    guard !base.isEmpty else { return }
    base.remove(at: base.startIndex)
}

The purpose of this function is to mutate the string by dropping the first character. Recall that this is a function on IntrepidStringExtension, not on String itself. That means that whenever we need to refer to a property or function of the string, we do so via base instead of self. The problem is that because String is a value-type, base is actually a copy of the original string. Changing the value of base will alter the copy, not the original string.

So whereas before we'd have something like:

var myString = "Intrepid"
myString.ip_dropFirst()
print(myString) // Prints "ntrepid"

Now we'd have:

var myString = "Intrepid"
myString.ip.dropFirst()
print(myString) // Prints "Intrepid". The string was not mutated

I do not know of any easy way around this. My opinion right now is that probably there is a way to allow .ip to mutate the string, but it would likely be very complex and hacky, and it would be fighting the basic value-type safety that Swift works so hard to enforce. Overall I think this is the biggest obstacle in the way.

Not all APIs make sense when transferred to the proxy style

For an example of this take a look at String+Indexing.swift in current Swift Wisdom. There are a few custom subscipt functions in there. We would need to leave these on the base String type. Otherwise it doesn't make much visual sense, because you'd have calls like this:

let substring = myString.ip[1..<myString.characters.count]

That said, while this is ugly in my opinion, I'm not sure what the best practices are around adding custom subscript implementations to an extension and ensuring that they don't conflict.

Also this is pretty subjective. In fact the originator of this pattern in RxSwift has a blog post specifically about using this to implement subscripts:

https://medium.com/@JegnuX/safe-collection-subsripting-in-swift-3771f16f883#.jrr86qail

I think the example there of implementing a safe subscript (like collection.safe[index]) makes more sense, because the word "safe" makes sense in the context of a subscript call, and it's arguably cleaner looking than the alternative collection[safe: index]. But for something like ip which is just a grab-bag of methods, I think it makes less sense. Maybe that's an issue with our overall naming, but that's a larger discussion.

Final thoughts

Overall I don't think working around these problems is worth the switch from .ip_ to .ip.. As far as I know, there aren't any major benefits that this change would buy us. Even if the base Rx setup worked as-is, it still adds an extra level of complexity that we could live without. At the end of the day, the entire organization needs to feel that they can easily jump into the Swift Wisdom codebase to make changes and put up PRs. With that in mind, I think our general rule should be to keep things as simple as possible.

[Benuuu]

Excellent research findings @andrewdolce! I agree with your conclusion.

[brightredchilli]

Hi @andrewdolce, really awesome and I'm glad you wrote this up. I have some follow up questions on each of these blockers.

1. Have you tried using the inout parameter? The reason that the .ip is not working on value types is because value types is, well, pass by value. Which describes the behaviour you are seeing exactly. I believe for this to work we need to change the constructor to be init(inout base: Base)
2. Maybe myString.ip[] is ugly, but it does prevent the conflict, in the event that Apple(or any third party developer) decides to implement the subscript. In the case of Apple, we would probably want to remove our own and use Apple's but in the case of the Third party developer, we reach a compile time impasse ... we really want to avoid being in that situation. That being said, if we don't migrate to .ip, I think we need to start changing our subscripts to be more safe.

[andrewdolce]

@brightredchilli Thanks for the comments. In response:

1. I can dig into this a bit more, but at this point my understanding is that using inout would allow us to pass the string by reference into the constructor, but doesn't offer a good way to capture that reference in the self.base property for later use. When we assign to self.base, it still makes a copy. But there may be something I've overlooked.
2. I agree that the subscript naming as-is could cause conflicts. Assuming we don't pursue ip., I'll give this some more thought and see if I can come up with an alternative recommendation.

[tzm41]

Just put another example that migrated to the dot syntax here for future reference.

https://github.com/SnapKit/SnapKit

[alexpersian]

@andrewdolce can the findings from this be documented somewhere and this issue closed?