Skip to content
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

Better optional property behavior and other enhancements to Swift support #245

Closed
wants to merge 6 commits into from

Conversation

beccadax
Copy link

@beccadax beccadax commented Sep 2, 2014

This pull request includes several enhancements to Swift support.

The main thing it does is give mogenerator a more sophisticated and nuanced understanding of Swift optionals. Commit 4a11e53's log message explains the rationale, but in short:

  • Optional properties are treated as Swift optionals (?).
  • Certain non-optional properties that can be nil before the object is first saved are treated as implicitly unwrapped optionals (!).
  • Non-optional properties that can't be nil unless you're doing weird, seemingly useless things are non-optional.

If you disagree with my rationale and think that mogenerator should allow Swift to handle all of Core Data's edge cases, I would still make non-optional properties implicitly unwrapped to reduce the cognitive burden on the programmer.

This pull request also includes two other small changes, which I apologize for including in an otherwise very focused branch:

  • The human file templates now include an import CoreData statement (cb09202). This is not strictly required for a managed object subclass in Swift, but it's necessary for many extremely common operations, such as creating fetch requests or taking managed object contexts as parameters, so it seems more convenient to include it.
  • Machine templates also include the <relationshipName>Set properties from the Objective-C templates, which access mutable proxies for relationships (48b1b9b and a39cddf). I've always found these incredibly handy.

If you don't like one of these, I won't be offended if you cherry-pick it out. Their commits don't include changes affecting the optional stuff.

Objects are guaranteed to be binary-compatible whether they're optional
or not, so this difference is invisible to Core Data.
Otherwise they can't use, for instance, NSManagedObjectContext.
In terms of both name and behavior.
The templates are pretty convoluted, and will get more so with the next
commit.
Until now, mogenerator's nil behavior has been an approximation of the
truth, because Core Data's "optional" is a bit different from Swift's
"optional". Basically, Core Data non-optionals can't be *saved* with
nil in them, while Swift non-optionals can't be *set* to nil at any
point.

There are four circumstances in which a "non-optional" Core Data
property can be nil:

1. If it is an attribute with no default value, or a to-one
relationship, the property will be nil upon initialization.

2. If a to-one relationship's inverse specifies a "nullify" rule, the
to-one relationship will temporarily become nil when the destination
object is deleted.

3. Objective-C code or setValue(_:forKey:) could set a property to nil
temporarily.

4. After an object is deleted and its context is saved, all of its
properties become nil.

To be fully correct, all Core Data properties would have to be either
optional (?) or implicit optional (!) in Swift. However, I don't think
3 and 4 are really worth worrying about very much. 3 involves
intentionally making a valid object graph invalid, and 4 involves
access to a permanently deleted object with no usable data. I don't
think either of these are worth losing much sleep over.

That leaves the first two cases. In 1, Core Data displays two-phase
initialization behavior. The recommended way of handling two-phase
initialization is an implicitly unwrapped optional. In 2, the nil
should probably be resolved by the same code that caused the problem,
so it should be very transient.

So the solution I took was to use implicit optionals to cover those two
cases, and ignore the other two. Properties marked optional in Core
Data are, of course, regular optionals in Swift as well.
@Isuru-Nanayakkara
Copy link

Hi @brentdax, does this fork resolves this issue #264? I think I'm referring to the fix you've done here. Am I correct?

@beccadax
Copy link
Author

beccadax commented Feb 6, 2015

@Isuru-Nanayakkara If I understand your issue correctly, this fork resolves it. All attributes and to-one relationships with the "optional" checkbox selected are marked as Swift optionals in this fork.

@Isuru-Nanayakkara
Copy link

Great! One question. How can I install your fork? Do you have a .dmg?

@beccadax
Copy link
Author

beccadax commented Feb 6, 2015

I don't have any mechanisms in place for installing it—I'm just looking to get it merged into the main project, and haven't given any thought to distributing it separately. One (hacky and untested!) way you could try doing it is to install Homebrew and then patch your formulas with this, but I haven't actually tried that.

@anilmanukonda
Copy link

1.29 treats every property as an optional. If this branch has a fix, it has to be merged asap.

@justin
Copy link
Collaborator

justin commented Dec 26, 2015

Can you chime in on #302, @brentdax? We are trying to modernize our Swift support

@justin
Copy link
Collaborator

justin commented Mar 22, 2016

We are declaring pull request and issue 0 now that 1.3 is out. If this is still an issue you'd like to see address with 1.30 and going forward, please open a new issue so we can start a fresh discussion. Thank you!

@justin justin closed this Mar 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants