3Squared / SQKDataKit

Lightweight Core Data helper to reduce boilerplate code.
MIT License
19 stars 6 forks source link

SQKCoreDataOperation properties clash with NSOperation #9

Closed blork closed 10 years ago

blork commented 10 years ago

SQKCoreDataOperation has two properties finished and executing that clash with identically names properties on NSOperation. This is a compiler error in Xcode 6/LLVM 3.5 and prevents the library building. It can be fixed by renaming the properties (see #8), but I'm not sure on the reasoning for having them at all vs. using the ones already defined. (@lukestringer90)

screen shot 2014-07-12 at 11 47 22

lukestringer90 commented 10 years ago

This is strange, not seen those warnings in Xcode 5 before. I wrote SQKCoreDataOperation using the example code from Apple on subclassing NSOperation here The only difference between them is that Apple's subclass uses raw ivars rather than properties.

(I think) the reason for these properties is to satisfy these requirements outlined in the subclassing notes:

Concurrent operations are responsible for setting up their execution environment and reporting the status of that environment to outside clients. Therefore, a concurrent operation must maintain some state information to know when it is executing its task and when it has finished that task. It must then report that state using these methods. Your implementations of these methods must be safe to call from other threads simultaneously. You must also generate the appropriate KVO notifications for the expected key paths when changing the values reported by these methods.

blork commented 10 years ago

Not sure why it wasn't an error before but it makes sense to me that it is - since the properties are named identically you aren't creating new ones but instead attempting to alter the semantics of the originals. The subclass is trying to change them from readonly to readwrite which should definitely not be allowed.

The subclassing notes tell you to override the already defined methods, not the already defined properties, so the solution in #8 seems best.

blork commented 10 years ago

I don't think we need the extra tracking of finished/etc. unless the NSOperation is set to concurrent (which ours isn't), and that all the processing takes place synchronously inside main (it could do, just needs moving from start). It'd simplify the code a bit if we could do that.

http://stackoverflow.com/questions/3859631/subclassing-nsoperation-to-be-concurrent-and-cancellable/4300849#4300849 http://nshipster.com/nsoperation/

blork commented 10 years ago

Though, thinking about it, if you want it to handle the merging itself like it does at the moment, then it does need to be concurrent.

Perhaps a simpler version that doesn't handle the merging? Though I could always make one myself.