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
Convert ORKResultPredicate into a class extension of NSPredicate #754
Comments
I have created a pull request for this this evening. Instead of using ORKResultPredicate, we have now created a category on NSPredicate called ORKResultPredicate. The syntax for creating predicates is tidier as a result - in Swift you generate predicates using NSPredicate(...) syntax; in Obj C you use [[NSPredicate alloc] initWith...] syntax which is much nicer. It will cause very minor API breakage to existing apps who will need to convert from the old syntax to this one - however as this is a 10 character change or so as demonstrated in ORKSample and ORKTest I'm hoping this is acceptable for the improved code readability. Best wishes, Andrew |
I've done further work on this over the weekend. The original pull request is named NSPredicate-Issue754, and should be ready for review. Following this I've been trying to make a more Swift-friendly interface for NSPredicate. I have had to do this in Swift; this is work in progress but I've published it as NSPredicate-GenericFuncs (the pull request title gives the clue as to why the code is in Swift!) I'm hoping the use of Swift is acceptable in this context. I have published it, but it is NOT ready yet for formal review and inclusion in the codebase. The Swift code for this feels cleaner to me - a much smaller set of convenience inits for min/max, expected, or matches. In Swift we have a very small number of convenience inits with much shorter syntax as we can use type inference. For objective C, we will need to retain the old syntax - so I'm rebuilding the old functions to just call the new Swift functions. Unlike NSPredicate-Issue754, Objective C code wouldn't see API changes for this issue; Swift ones could use the old functions but would be advisable to change to the new syntax which is much shorter and extensible. I am however struggling a little with the choice question results - opinions and advice welcome here.
|
I've now put three separate PRs relating to predicates up.
|
I've done some work on this today, effectively merging the latter two together, and making a number of tidy-ups. The objective C initialisers are marked as Obj C-only, and in Swift (2.2) we have a set of convenience inits utilising generics replacing all the initialisers. I've temporarily implemented NSDate as comparable in there; when Swift 3 is ready we can change to using Date which is Comparable by default. |
I was initially looking at the code with regard to making some features Swift-friendlier.
When I looked at ORKResultPredicate.h, the thought occurred to me that the ORKResultPredicate class is actually just initialising a set of NSPredicates.
Therefore, I have a very simple suggestion - it will be an API change but hopefully one which makes things easier for everyone.
Why not make all the publicly exposed functions in ORKResultPredicate initialiser functions on a class extension to NSPredicate, and retire ORKResultPredicate altogether?
The function names would probably rename to initForTimeOfDayQuestionResult...
If we needed to maintain backwards compatibility for a bit we could just have a set of boilerplate code functions still residing in an ORKResultPredicate class whilst it was deprecated (not sure what the thoughts are re. deprecation of APIs in ORK).
If agreed this would be helpful, happy to generate the PR for it.
The text was updated successfully, but these errors were encountered: