Typesafe predicates

[Revolucent]

Here are the features I'd like to see for type-safe predicates.

• Support for any type conceivably supported by Core Data, especially the myriad of numeric types, both as literals and variables.
• Type-safety should be optional (but the default). I.e., the old "loose" comparisons should still work, and should always be available as a fallback.
• Transformables would be a nice bonus, but not required.

[patgoley]

@Revolucent One thing to clarify about the 2nd requirement. If we're generating typesafe attribute descriptions, we wouldn't want to inadvertently fallback to a "loose" comparison because a caller used an invalid type. So, if we want to opt for type safety by default and with the same operators, I don't see a way we can allow the loose comparisons to continue working in that scenario.The only impact on existing code is that invalid comparisons currently generating runtime errors become compile time errors, which I think is a plus.

[Revolucent]

@patgoley That's not quite what I meant by "fallback". Either you have type safety or you don't. Instead, I meant that we should allow the existing non-typesafe comparisons to work as an option. In other words, type safety should be an opt-out default, but once you've opted in (for a particular model attribute), you're in.

Right now, the comparison operators are implemented in terms of CustomExpressionConvertible, which Attribute implements. I could imagine two subclasses of Attribute, one that implements your TypedExpressionConvertible and one that does not. Each would have a separate set of operator overloads that feeds into CustomExpressionConvertible. The advantage of this is that it provides a kind of "escape hatch". Users could pass a flag to cdqi to disable typed expressions, or they could simply edit the generated proxy.

As for the numeric types, I think I was overzealous. A better way to do it would just be to support the numeric types supported by Core Data (NSNumber, Float, Int32, Int64, Double, etc.), and let the user do any necessary conversions. (In other words, we should do it the way you probably intended.)

[patgoley]

@Revolucent Makes sense about the fallback option, I'll see what I can do. Just to be clear, do we want to be able to opt in per attribute? I think it might be a bit confusing to mix typesafe and non typesafe attributes on a model, possibly leading you to think you're being safe when you're actually not.

I may have actually come up with a solution to handle any numeric type without unnecessary casting. By creating a single NumericAttribute class with ValueType = NSNumber, swift will intelligently coerce many primitive types (Int, UInt, CGFloat, Double) into NSNumber for comparison. For other types that won't coerce, I've made a protocol NumericValueType which allows a primitive to create an NSNumber version of itself using the proper constructor, which is then used for comparison. This allows for any numeric type to work and will correctly compare values of different types without requiring a manual cast or precision loss.

[Revolucent]

For the fallback stuff, what I meant was that implementers could fallback manually by editing the attribute proxy, or they could specify a cdqi option that nukes all type safety.

But perhaps I'm being overcautious. I'm trying to think of a reason we'd need to preserve loose typing. I'm not married to it, and I can tell you don't have much enthusiasm for it. Why don't we just skip loose typing and go all-in with type safety? We can release this with CDQI 4.0.

[Revolucent]

I have some major projects for corporate clients that are using CDQI. (These are not in the App Store.) While unit tests are awesome, the real test of this change will be how well those projects compile and run after the new attribute proxies are generated. In theory, very few code changes should be needed.

[patgoley]

I totally understand the need for backwards compatibility, but if we can achieve full type safety without public API changes, the only code that will break on upgrade are places where invalid type comparisons were made, no refactoring would be necessary. At least, that's what I'm hoping to achieve with the updates I'm making.

You're right, my enthusiasm for the unsafe way is lacking :P As you mentioned in the original issue, it doesn't live up to the value proposition for the framework. I'm happy to continue supporting it if there are breaking changes introduced, though I do think it should be deprecated at some point.

[Revolucent]

Let's roll the dice and drop loosely typed comparisons. I'm sold. Out of curiosity, are you using CDQI in any "real" projects?

[patgoley]

Awesome. At this point I haven't used it on a real project, but I do have plans to. I'm happy to let you try out my branch on one of your big projects before we merge to determine the impact. I expect the PR will be ready in the next few days.

[patgoley]

@Revolucent Progress is coming along nicely here. If you check out the typesafe-attributes branch on my fork you'll see there is working type safety for primitive types and relationships, both to-one and to-many. However, it's now apparent that we also need to limit certain comparison functions and operators to certain types. For example, you shouldn't be able to call greaterThan on a BooleanAttribute, which is currently allowed.

Basically, we need to take the extension on CustomExpressionConvertible with equalTo, greaterThan, etc and break it down into multiple protocols. Something like EquatableAttribute and ComparableAttribute (or perhaps it should be on the value types, not sure yet). Here's how I'm picturing the breakdown but I wanted your input.

Equatable attributes have equalTo, notEqualTo, among
Boolean
Binary

Comparable attributes have lessThan, greaterThan,between and are also Equatable
String
Numeric
Date

only String attributes should have:
beginsWith
endWith
matches
like
contains

Collection attributes (to many relationships) should also have contains. Collection attributes will also be an equatable attribute but the resulting predicate needs to be ALL employees IN x and not employees == x (reason why one test is currently failing)

I hope that makes sense. I really just wanted your take on which comparison functions should be available for which types. Thanks!

[Revolucent]

I think you're on the right track here. I'd avoid putting ALL in a generated predicate. Just use the global all function for that, but obviously it needs to be IN instead of ==, as you noted.

I'm spending some quality time with your code right now, and will be looking over it, stepping through it, examining it, and perhaps even having a glass of wine with it over the next hour or two until my wife gets home. I'll let you know my thoughts.

[Revolucent]

@patgoley It's a shame that Int16, UInt16, Int32, etc. are not autoboxed to NSNumber. (An oversight by the Swift team or a deliberate choice?) Then you wouldn't have to have two overloads of **.

[patgoley]

@Revolucent It is a shame. It's too bad there isn't an inverse of IntegerLiteralConvertible that we can extend it ourselves. I'm trying to figure out if there's a better way to convert the primitives to AnyObject so the already class types don't have to implement boxedValue. On some level you could check if x is AnyObject or is ExpressionValueType but you can't really use an OR on the associatedType of TypedExpressionConvertible.

[Revolucent]

@patgoley

Your Code

Alright, I didn't need to spend that much time with this code. It's really very simple. A TypedExpressionConvertible has an associated ValueType which must implement the ExpressionValueType protocol. ExpressionValueType is a type that knows how to convert itself into a boxed value. When building predicates, you simply ask for the boxed value and use it to construct the predicate.

Numeric comparison rely on the (unfortunately not universal) capability of Swift to autobox some common numeric types to NSNumber. There's a fallback for those types (like Int32) that don't do this.

Operators / Comparison Protocols

I agree with your choices. Obviously, as you suggested CustomExpressionConvertible's extension methods should be broken apart. Actually, that's not quite what I'd like to see. Instead, I like to see something like the following:

extension ExpressionValueType: CustomExpressionConvertible {
    var expression: NSExpression {
        return NSExpression(forConstantValue: boxedValue)
    }
}

public struct PredicateBuilder { // or some useful name
    public static func compare(lhs: CustomExpressionConvertible, rhs: CustomExpressionConvertible?, type: NSPredicateOperatorType, options: NSComparisonPredicateOptions = []) -> NSPredicate
        let le = lhs.expression
        let re = rhs?.expression ?? NSExpression(forConstantValue: nil)
        return NSComparisonPredicate(leftExpression: le, rightExpression: re, modifier: .DirectPredicateModifier, type: type, options: options)
    }
   // Use compare to build the rest of the needed methods, like equalTo, greaterThan, etc.
}

You would then use this public, generalized PredicateBuilder struct and its static methods to implement your protocol methods and operators. It would also be available to users of CDQI. (And provides my "loosely typed" escape hatch to boot.)

I'm happy to write this struct if you'd like. I can get it done in probably a few hours.

ExpressionValueType

Not sure about this name. Its real purpose is to "box" something. So I prefer one of the following:

protocol ExpressionValueType {
    var expressionValue: AnyObject { get }
}

or

protocol BoxedValueType {
    var boxedValue: AnyObject { get }
}

I think the first one fits a little better with the names used in the rest of the project, but I'm open to either one.

[Revolucent]

So, this is what I had in mind. This code is pretty off the cuff and has never been compiled or run by anyone ever:

protocol CustomExpressionConvertible {
    var expression: NSExpression { get }
}

struct PredicateBuilder {

    static func compare(lhs lhs: CustomExpressionConvertible, rhs: CustomExpressionConvertible?, type: NSPredicateOperatorType, options: NSComparisonPredicateOptions = []) -> NSPredicate {
        let le = lhs.expression
        let re = rhs?.expression ?? NSExpression(forConstantValue: nil)
        return NSComparisonPredicate(leftExpression: le, rightExpression: re, modifier: .DirectPredicateModifier, type: type, options: options)
    }

    static func greaterThan(lhs lhs: CustomExpressionConvertible, rhs: CustomExpressionConvertible, options: NSComparisonPredicateOptions = []) -> NSPredicate {
        return compare(lhs: lhs, rhs: rhs, type: .GreaterThanPredicateOperatorType, options: options)
    }

}

protocol ExpressionValueType: CustomExpressionConvertible {
    var expressionValue: AnyObject { get }
}

extension ExpressionValueType {
    var expression: NSExpression {
        return NSExpression(forConstantValue: expressionValue)
    }
}

extension String: ExpressionValueType {
    var expressionValue: AnyObject {
        return self as NSString
    }
}

protocol ComparableValueType: ExpressionValueType {}

protocol TypedExpressionConvertible: CustomExpressionConvertible {
    associatedtype ValueType: ExpressionValueType
}

extension TypedExpressionConvertible where ValueType: ComparableValueType {
    func greaterThan(other: ValueType) -> NSPredicate {
        return PredicateBuilder.greaterThan(lhs: self, rhs: other)
    }
}

func ><E: TypedExpressionConvertible, V: ComparableValueType where E.ValueType == V>(lhs: E, rhs: V) -> NSPredicate {
    return lhs.greaterThan(rhs)
}

[Revolucent]

@patgoley

Ah, another thing that just occurred to me, and that unfortunately is not reflected in the unit tests, but is currently supported: You must be able to compare attributes to each other, e.g.,

moc.from(Foo).filter { $0.bar >= $0.baz }

So far, it doesn't look like your additions will support that. I admit doing this is rare, but it's a must-have.

[Revolucent]

Never mind. It's actually pretty easy:

extension TypedExpressionConvertible where ValueType: ComparableValueType {
    func greaterThan(other: ValueType) -> NSPredicate {
        return PredicateBuilder.greaterThan(lhs: self, rhs: other)
    }
    func greaterThan(other: Self) -> NSPredicate {
        return PredicateBuilder.greaterThan(lhs: self, rhs: other)
    }
}


func ><E: TypedExpressionConvertible, V: ComparableValueType where E.ValueType == V>(lhs: E, rhs: V) -> NSPredicate {
    return lhs.greaterThan(rhs)
}

func ><E: TypedExpressionConvertible where E.ValueType: ComparableValueType>(lhs: E, rhs: E) -> NSPredicate {
    return lhs.greaterThan(rhs)
}

[patgoley]

@Revolucent Thanks Gregory, I appreciate the feedback.

I had actually begun a refactor on the comparison functions before you posted and wanted to run it by you. Instead of the PredicateBuilder, I had basically added extensions to TypedExpressionConvertible so we can leverage the ValueType (see below).

I like the abstracted nature of PredicateBuilder, allowing direct access to the functionality, but with TypedExpressionConvertible extensions, we can make sure that operators are only applied to matching and appropriate values. I recognize the value of flexible helpers underneath the strictly-typed public API, but I think once we have the very general compare function, all operator specific functions should be type constrained.

extension NSNumber: ComparableValueType { }

extension TypedExpressionConvertible where ValueType: ComparableValueType {

    public func greaterThan(rhs: ValueType?, options: NSComparisonPredicateOptions = []) -> NSPredicate {

        return compare(rhs?.boxedValue, type: .GreaterThanPredicateOperatorType, options: options)
    }

    ...
}

extension TypedExpressionConvertible where ValueType == String {

    public func beginsWith(rhs: String, options: NSComparisonPredicateOptions = []) -> NSPredicate {

        return compare(rhs, type: .BeginsWithPredicateOperatorType, options: options)
    }

    public func contains(rhs: String, options: NSComparisonPredicateOptions = []) -> NSPredicate {

        return compare(rhs, type: .ContainsPredicateOperatorType, options: options)
    }
}

In this fashion, we don't expose an API that allows you to make mistakes. PredicateBuilder, while flexible and a solid abstraction, could allow you to compare invalid types or use an invalid operator on certain types. If we want PredicateBuilder, I don't think it should be public on the package level.

About the error you found comparing attributes, I just actually ran across that same limitation in a different form. The problem I ran into actually was not being able to compare a countable.count > 0 complaining because the result of countable.count is an NSExpression and didn't meet the type constraints of > which expected a numeric or comparable type. I think we can get around this, basically switching to TypedExpressionConvertible in a few places that CustomExpressionConvertible or NSExpression are used right now. I don't think it should be too difficult.

Thanks for your help Greg! Hoping to get some more done this weekend. Let me know if you have any thoughts or ideas.

[patgoley]

Ah it seems we arrived at the same conclusion but I didn't see your post until now. Yeah, the extensions on TypedExpressionConvertible are nice. Still have to see how to handle the countable expression result and attribute to attribute comparisons.

[Revolucent]

I like the abstracted nature of PredicateBuilder, allowing direct access to the functionality, but with TypedExpressionConvertible extensions, we can make sure that operators are only applied to matching and appropriate values.

Well, my suggestion actually is that we have both. In other words, the TypedExpressionConvertible extensions should be implemented in terms of PredicateBuilder.

[patgoley]

I'm good with that idea. It makes the predicate building nice and testable without having to create expressions first.

[patgoley]

My branch is to date with everything minus PredicateBuilder and the errors I mentioned. Not sure where you have that living right now but I'm happy to merge it into my fork whenever it's ready. Or we can publish this feature branch on the main repo and you can merge it there. Whatever works for you.

[Revolucent]

I can refactor this code to use PredicateBuilder. Does this code support attribute comparisons? If not, I can add that, too.

It looks like it does not.

[patgoley]

Great, thanks. It doesn't support attribute to attribute comparisons at this point. I'm also a bit stuck on the countable and subquery function results. We somehow need to know those are numeric value expressions and comparable to other numbers. One of the tests isn't compiling at the moment for that reason.

Anyway, thanks for the help. I'm probably done for tonight but I think if we can round out these edge cases we'll be pretty close to finishing.

[Revolucent]

Or we can publish this feature branch on the main repo and you can merge it there.

Yes, this is what I'll do. I'll add PredicateBuilder to it. I may also try my hand at countable and the subquery function results.

[Revolucent]

@patgoley I've copied your feature branch over to the main repo. I'm still doing some work in it. I want to warn you that I've made some pretty extensive changes, so you might want to wait a bit before continuing any work.

I've changed the TypedExpressionConvertible protocol as follows:

protocol TypedExpressionConvertible: CustomExpressionConvertible {
    associatedtype ExpressionValueType
}

Note that…

1. I've renamed ValueType to ExpressionValueType.
2. I've eliminated the ExpressionValueType protocol which was used for boxing.
3. And thus, I've eliminated boxing.

Why? Boxing is unnecessary. It comes from a misunderstanding of how CDQI and Core Data predicates work under the hood. Core Data predicates want to compare NSExpressions and nothing else. These expressions can be constant values, key paths, whatever, but ultimately, they will become NSExpression. The only thing a TypedExpressionConvertible needs to do is produce an NSExpression.

So have I thrown type safety out the window? No. My way of doing it is just as typesafe as yours. The key is the ExpressionValueType associated type. Any two types that implement TypedExpressionConvertible and have the same ExpressionValueType can be used together to build predicates. What this means is that types like String, rather than implementing ExpressionValueType (which is now gone), implement TypedExpressionConvertible directly. This way there is only one protocol to deal with: TypedExpressionConvertible. This also neatly solves the problem of comparing attributes.

Here's some code that should make it all clear:

extension String: TypedExpressionConvertible {
    public typealias ExpressionValueType = Self
    public var expression: NSExpression {
        return NSExpression(forConstantValue: self)
    }
}

extension TypedExpressionConvertible where ExpressionValueType: Equatable {
    public func equalTo<R: TypedExpressionConvertible where Self.ExpressionValueType == R.ExpressionValueType>(rhs: R?, options: NSComparisonPredicateOptions = []) -> NSPredicate {
        return PredicateBuilder.equalTo(lhs: self, rhs: rhs, options: options)
    }

    public func notEqualTo<R: TypedExpressionConvertible where Self.ExpressionValueType == R.ExpressionValueType>(rhs: R?, options: NSComparisonPredicateOptions = []) -> NSPredicate {
        return PredicateBuilder.notEqualTo(lhs: self, rhs: rhs, options: options)
    }

    public func among<R: TypedExpressionConvertible where Self.ExpressionValueType == R.ExpressionValueType>(rhs: [R], options: NSComparisonPredicateOptions = []) -> NSPredicate {
        return PredicateBuilder.among(lhs: self, rhs: rhs.map { $0 as CustomExpressionConvertible }, options: options)
    }
}

public func ==<L: TypedExpressionConvertible, R: TypedExpressionConvertible where L.ExpressionValueType == R.ExpressionValueType, L.ExpressionValueType: Equatable>(lhs: L, rhs: R?) -> NSPredicate {
    return lhs.equalTo(rhs)
}

let predicate: NSPredicate = department.name == "foo"

Autoboxing of NSNumber still works with this technique. (NSNumber implements TypedExpressionConvertible.) Also, I was able to eliminate the ComparableValueType and EquatableValueType protocols and just use Comparable and Equatable.

This code is not pushed yet. It still has a few minor issues, but I'm too tired to solve them right now.

[patgoley]

@Revolucent This is great! I think the use of TypedExpressionConvertible for the constant values makes total sense. The only reason I wanted the boxed value was so it could be passed to NSExpression(forConstantValue:), but this makes that requirement more formalized.

I had thought about using Swift's Comparable and Equatable, but I found that there wasn't a 1 - 1 relationship between types that are Comparable by the protocol and types that can be compared with NSExpression. Namely, NSDate is not Comparable. I'm not sure that as a framework we should make NSDate conform to Comparable just to make this work for two reasons:

1. We don't actually need an implementation for Comparable, since the comparison is done by NSExpression / NSPredicate. This would be confusing to require a type to implement a function that's not ever called in the framework (since we override it with the predicate generating operators).
2. It may conflict in applications where users have already extended NSDate to conform to Comparable. I had created ComparableValueType as an empty protocol just so we could indicate which types are comparable with CDQI and not rely on conformances to protocols which we don't control.

There's still a bit of work to be done. Need to update cdqi to generate EntityCollectionAttributes for to-many relationships and need to handle Transformable types (stubbed out in a comment in TypedExpressionConvertible). I think I can get to these items at some point today, but let me know when you've pushed your changes so I can grab them. Thanks!

[patgoley]

P.s. I guess to handle the myriad numeric types now, we just make them all have NSNumber for the ExpressionValueType. You probably figured that out already.

extension Int32: TypedExpressionConvertible {
    public typealias ExpressionValueType = NSNumber
    public var expression: NSExpression {
        return NSExpression(forConstantValue: NSNumber(long: self))
    }
}

[Revolucent]

Yes, I've already got all the comparisons working very well. As for Comparable, Equatable and their relationship to NSDate and NSNumber, keep in mind that the ExpressionValueType can be any arbitrary type. No instances of this type are ever instantiated or compared. It's really just a kind of "key": Any two TypedExpressionConvertibles that have the same ExpressionValueType can participate in comparisons. So, I've replaced NSNumber with Int for numerics. As for dates, well…

public struct Date: Equatable, Comparable {
}

public DateAttribute: TypedExpressionConvertible {
    public typealias ExpressionValueType = Date
}

extension NSDate: TypedExpressionConvertible {
    public typealias ExpressionValueType = Date
}

This Date type is just a key, and a place to hang Comparable and Equatable. Since no instances of it are ever created, its implementation is irrelevant.

[Revolucent]

I'm watching my two-year-old until around 2PM EST. Around 5 PM, my old Gen X ass is going to see Duran Duran. But between those times, I'm confident I'll be able to push some code for you to work with.

[Revolucent]

@patgoley

OK, I'm very close to pushing my changes. A few things to note:

EntityCollectionAttribute

Unfortunately I had to get rid of this class and go back to what I was using before. The reason is that EntityCollectionAttribute didn't permit the building of perfectly valid Core Data queries such as these:

ANY employees.lastName == 'Goley'
any(department.employees.lastName == "Goley")

The reason is that EntityCollectionAttribute does not have a lastName property. If you can think of a way to salvage this, I'm certainly open to it, but I can't see one that isn't unnecessarily complex. I don't much like the idea of building an alternate set of attributes just for collections.

At some point, we have to keep in mind that CDQI is reflecting what Apple's predicate language allows underneath the hood, and I think it should do it in as seamless a way as possible. The only "violation" of type safety here is that you could perform subquery operations on a non-collection type. I'm not terribly worried about that. Any developer who uses subquery frequently is more than likely to know what they are doing.

[Revolucent]

@patgoley OK, it's pushed. Have a look and let me know what you think.

[Revolucent]

@patgoley OK, just pushed some last minute changes. Had to fix up cdqi.

[Revolucent]

I think the only thing missing is transformable attributes.

[Revolucent]

@patgoley

Actually, Pat, I think I've got this sewn up. I'd really love for you to look at it and give your feedback. I admit I feel a tad guilty that I effectively rewrote what you did. (Hope you're not too disappointed. I tremendously appreciate the initiative you took on this.) On the other hand, I really think this is the right approach. I'm a little iffy on a few things, and there are a few oddities with the numeric types, although all the unit tests pass.

I'm not worried about transformable attributes, though if you want to tackle them, awesome. I will probably push those in a point release.

I'm going to tinker with this a bit more to tighten it up, and I encourage you to do the same. But I'm going to create a release-4.0 branch and merge this code into it within the next few days, though I'm not quite ready to release v4.0 yet.

[patgoley]

@Revolucent No worries, Greg. As far as rewriting it, as long as my name is in the commit history, I have no complaints. I appreciate the opportunity to contribute to a great framework and I honestly don't care who gets the last word on this feature, so to speak. I just wasn't about to let you rewrite your readme instead of improving your code :)

I would love to take a thorough look over it before it gets merged in, just to make sure I don't find any gotcha cases etc. Are all of your latest commits pushed up? From what's up there now it doesn't seem quite done, and I'm not 100% sure on all of the choices yet. Here's a few things I noticed:

1. It seems like you nixed the EntityCollectionAttribute for handling to-many relationship comparisons, did you have an alternative plan for that?

2. I feel weird about having to create so many empty structs (Date, Entity, Null) just for associated type comparisons, and then make them conform to real protocols whose functions don't matter in this context. It just seems like a misuse of the protocols that makes us create a lot of boilerplate (empty types and operators for them) to avoid affecting the real types (ie we made Date so we didn't have to conform NSDate to comparable). We're required to write code that doesn't do anything, which seems like a bad practice.

The other way that we had before I think made more sense and required less boilerplate. We have empty protocols such as ComparableValueType which are only used to constrain the appropriate functions to types that we've "tagged" as such. We can add these to real types (NSDate, NSNumber) without possibly interfering with anyone else's code.

As far as Null goes, it's really unfortunate that we have to duplicate anything that could possibly take Null. Was there some limitation not allowing us to use an optional CustomExpressionConvertible? and then convert using NSExpression(forConstantValue: nil) at the last second inside of compare? This would seem much more natural to me and would remove a lot of function overrides.

3. EntityAttribute is a bit weird, it only takes the associatedtype so far to say it's an Entity. In my implementation, DepartmentAttribute was a TypedExpressionConvertible where ExpressionValueType == Department. I don't think we have the same level of type safety anymore since department attributes could be compared to any other Entity.

[Revolucent]

@patgoley I don't have a lot of time to answer this, so I'll just jot a few notes. Later tonight I can answer more fully, if needed.

1. I nixed EntityCollectionAttribute for reasons I gave above. The problem with it is that it did not support Core Data expressions such as ANY employees.lastName == 'Goley'. When programming in CDQI, it's important to think from NSExpression and NSPredicate up, rather than from Swift down, if you catch my meaning.
2. It feels weird, but it works beautifully. The other way may have required less boilerplate, but didn't solve certain fundamental problems. Remember, CDQI must ultimately produce NSExpressions and NSPredicates. Boxing simply does not matter. Or rather, the "box" that you need to create is NSExpression, not AnyObject.

The problem with your approach is that it doesn't get us all the way there. For instance, take a look at how I implemented Countable. Ultimately, what we need to return for a countable is something like %@.@count, where '%@' is another NSExpression. If we used your approach, what would we box? It was actually attempting to solve that problem with your approach that made me realize it just would not work. Once you realize that we are trying to create NSExpression instances, and once that makes you realize that you have to throw out boxing, I can't think of what other approach we would use.

Null is unfortunate, but I could not get any other way to work. You're welcome to try. (And no, there is no kind of optional that will work. The reason is that Swift ultimately needs a concrete type, but an optional CustomExpressionConvertible is not a type, so it cannot reify the signature of the method.)

1. It is actually legitimate in Core Data to compare disparate entities. For instances, I can say, "employee == %@", department. It may be rare, but it is completely legit. It's especially important when you have inherited Core Data types.

[patgoley]

@Revolucent Tell Duran Duran you have important work to do! Just kidding, have fun.

Sorry I missed your post on EntityCollectionAttribute, that makes sense. Those kind of queries did work but only by building it with subquery using the Aggregable type. I've expressed the direct comparison case in a unit test to make sure it always compiles :)

All great points above, I've demonstrated them to myself and they makes sense. The main thing my implementation was missing was applying TypedExpressionConvertible to the value types themselves, things follow pretty naturally from there.

There's one thing I've updated from your code (on my fork) and I think it's an improvement, but I'll let you decide...

I really don't think we need the intermediate types Data, Date, and Entity. I also think it's not ideal that we're conflating Equatable and Comparable to our own unrelated use case. Just because a type is Comparable does not necessarily mean NSPredicate knows how to compare it, and vice-versa. In fact, whether or not a type is Comparable is irrelevant to CDQI, because we don't ever call the related operators (only our overridden ones that create NSPredicates). Also, our implementation assumes that String is Comparable and always will be, but if this changes for whatever reason (they removed ++ for christ sake), CDQI will stop compiling (even though it would actually still work at runtime).

For this reason, I suggest we return to the previous idea of using our own, empty protocols and apply them to real types (instead of applying real protocols to empty types). I've pushed a working example to my fork that doesn't include Date, Data, or Entity structs and still works just the same. See below:

public protocol EquatableExpression { } //frankly not even necessary since all expressions are considered equatable

public protocol ComparableExpression: EquatableExpression { }

//DateAttribute.swift

public class DateAttribute: KeyAttribute, TypedExpressionConvertible {
    public typealias ExpressionValueType = NSDate
}

extension NSDate: TypedExpressionConvertible, ComparableExpression {
    public typealias ExpressionValueType = NSDate
    public var expression: NSExpression {
        return NSExpression(forConstantValue: self)
    }
}

//Operators.swift

public func ><L: TypedExpressionConvertible, R: TypedExpressionConvertible where L.ExpressionValueType == R.ExpressionValueType, L.ExpressionValueType: ComparableExpression>(lhs: L, rhs: R) -> NSPredicate {
    return lhs.greaterThan(rhs)
}

So what's different?

1. We don't need to create an empty Date struct just to link DateAttribute to NSDate. We can link the real types directly and tag them with one of two empty protocols to constrain them to the correct comparison functions. It's less code for each type involved.
2. We don't need to implement a totally unused operator just so we can leverage Equatable and Comparable. While those protocols are conceptually the same as what we're doing, in reality they mean something very specific: implements == and > which return Bool (not NSPredicate). Just because a developer made his or her type Comparable doesn't bring it any closer to being Comparable in the same sense by CDQI.
3. We can "tag" the appropriate types for comparison operators with our own empty protocols, letting us express things in our own terms. If Swift no longer supports Comparable on String as part of the standard lib one day, nothing breaks (contrived example but true).

I understand now why the Null struct is necessary. Swift won't recognize the type conformance on nil to the protocol in question because it doesn't know what T is on the Optional. We need a concrete, NilLiteralConvertible type that conforms to trampoline off of. This is a special case, other types don't require the intermediate type to work properly.

I hope that makes sense. If we can get by in all scenarios with out the intermediate structs, I think we should toss them. Let me know if you find a case where this doesn't work.

[Revolucent]

@patgoley I'm back. Your message popped up on my Apple Watch between sets, so I was pondering what you wrote while Simon Le Bon was belting out the ancient pop classics from my early teenage years.

So, with the caveat that I haven't looked at your code yet—I'm old as fuck, somewhat inebriated, and pretty tired, so that will have to wait until sometime Sunday—let me sum up what I like about both of our approaches.

The difference between your approach and mine can be summed up as: You don't want new concrete types and I don't want new protocols. Both solutions are mildly ugly, but c'est la vie in software. I'm much less concerned than you are that something like String would cease to be Comparable, but I guess it's (very, very remotely) possible. Actually, I'm not concerned at all about it.

So, I rather like the fact that I'm using Equatable and Comparable, even if we never call any methods on them. It seems kind of tidy to me, and even a bit clever. What I don't like (and you really don't like), is that I have to create these "key types" (as I call them in my head). Well, we still need key types, of course, you just prefer them to be something more "natural", even though they are still only used as keys in your approach just as they are in mine. No instances of those key types are created by virtue of their being key types.

But…I think what may have sold me on your approach is that it's less code to write. Just some empty protocols and that's it. So, with the proviso that I still need to review your code, I think we'll go with your approach.

And now I'm going to crash.

[Revolucent]

Oh, there is another thing I like about your approach, which I meant to mention. I do prefer that the types are more "natural", even if they are never used as such.

[Revolucent]

@patgoley

OK, I took a look at your code this morning and I'm sold. I do prefer your approach. However, in the interest of parsimony, let's get rid of the EquatableExpression protocol. As you mentioned, everything is equatable by default, otherwise it can't participate in predicates at all.

This leaves us with just the empty ComparableExpression protocol, which has no members. Now I'm even more sold on your approach. I love minimalism.

[Revolucent]

OK, I integrated your changes, removed EquatableExpression, and pushed to the main repo. I could be persuaded to revert the EquatableExpression commit if you have strong feelings about it.

[Revolucent]

I deleted the typesafe-attributes branch, created release-4.0, and merged those changes into it.

[patgoley]

Awesome! No I agree, if everything is considered equatable it's just a pain to tag everything as such. Glad it's merged in. Did you get a chance to run it on of you bigger projects? Seems like once you run cdqi again it should "just work".

I hope Duran was awesome and you were dancing on the sand.

[Revolucent]

I'll try it on the most complex project today and let you know what happens.

[Revolucent]

@patgoley

This is a large project that makes VERY extensive use of CDQI, so to have so few errors is excellent. I will probably edit CDQI in situ alongside this project until the errors go away. Then I will write some unit tests to cover these cases.

[Revolucent]

Actually, most of them were fixed by making ReadingAttribute Aggregable, which somehow got left out of cdqi. Easy fix.

[Revolucent]

I hope Duran was awesome and you were dancing on the sand.

Duran was awesome. For guys in their late 50s, they were absolutely rock solid. They're all so rich they could have hung it up long ago, so they obviously love what they do, and it shows. I actually don't listen to Duran much, but I didn't want to pass up the chance to see them. My musical taste these days tends more towards Röyksopp and Purity Ring, musical nephews and nieces of Duran.

[Revolucent]

@patgoley

I've squashed all of the compilation errors with this project, and everything works fine. However, one thing I noticed is that Swift's autoboxing of certain numeric types to NSNumber seems to interact unreliably with CDQI's typed comparisons. Not sure why. The solution seems to be that we simply have to implement TypedExpressionConvertible explicitly on types such as Int. When I do this, all of the strange edge cases go away.

Another huge win here is the ability to make any type participate in CDQI comparisons. This is especially useful with enumerations, e.g.,

extension Weekday: TypedExpressionConvertible {
    public typealias ExpressionValueType = NSNumber
    public var expression: NSExpression {
        return NSExpression(forConstantValue: rawValue)
    }
}

[patgoley]

@Revolucent That's awesome. Glad you were able to fix the edge cases and get enum support! I'm going to take a look later tonight about updating the unit tests and readme and hopefully we can get this release merged in soon. Nice work!

[Revolucent]

@patgoley Looks like we wrapped this one up. Thanks enormously for your help.