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

Adds Nullability #399

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Adds Nullability #399

wants to merge 2 commits into from

Conversation

fabb
Copy link

@fabb fabb commented Oct 31, 2016

No more make? inside blocks.

self = [super init];
if (!self) return nil;
if (!self) return self;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a bit awkward, i know, but otherwise the compiler will complain that the method should not return nil due to nullability.

@@ -14,6 +14,8 @@
*/
@interface MASViewAttribute : NSObject

NS_ASSUME_NONNULL_BEGIN

/**
* The view which the reciever relates to. Can be nil if item is not a view.
*/
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, actually the view can be nil according to the comment, need to fix this in a new commit

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, fixed it. fixed also the designated initializer which allows view to be nil too. did not fix the convenience initializer, as view ist used for both view and item there, and item may not be nil.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually now did fix the convenience initializer, as the item property is weak and thus needs to be nullable

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i mean, it could be forced that the parameter is non-nil in the initializer, and the property still can become nullable - but as the MASConstraintDelegateMock explicitly passes nil, I went with making it nullable.

@Panajev
Copy link

Panajev commented Oct 31, 2016

In this case either you change the method to expect a nil return value or you throw an exception there...

Both options may not be viable, but I personally find it very dangerous to have methods disobey their public API contracts. The compiler is right in this case.

Sent from my iPhone

On 31 Oct 2016, at 07:48, fabb [email protected] wrote:

@fabb commented on this pull request.

In Masonry/MASCompositeConstraint.m:

 self = [super init];
  • if (!self) return nil;
  • if (!self) return self;
    this is a bit awkward, i know, but otherwise the compiler will complain that the method should not return nil due to nullability.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@fabb
Copy link
Author

fabb commented Oct 31, 2016

@Panajev:
Actually Apple's default implementation of init looks like this:

- (id)init {
    if (self = [super init]) {
        // code
    }
    return self;
}

This is exactly the same - also returns self when !self.

Patrick Pöchhacker and others added 2 commits October 6, 2017 16:09
…ies which cannot be nullable and other parameters that are explicitly allowed to be nil from the context
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.

None yet

2 participants