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

Add NS_DESIGNATED_INITIALIZER to ASViewController initWithNode: #1054

Merged
merged 2 commits into from
Aug 5, 2018

Conversation

maicki
Copy link
Contributor

@maicki maicki commented Jul 29, 2018

This may introduce an API change in the ASViewController and changes in client code is needed but I will put that up for now.

Copy link
Member

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

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

Pretty harmless. The migration pathway here is straightforward.

@maicki maicki force-pushed the MSASViewControllerDesignatedInitializer branch from a451b34 to c83c428 Compare August 3, 2018 14:17
@maicki maicki force-pushed the MSASViewControllerDesignatedInitializer branch from d10c2a2 to d216bcc Compare August 5, 2018 14:31
@maicki maicki merged commit 847884a into master Aug 5, 2018
@haashem
Copy link

haashem commented Aug 7, 2018

By doing so, and initiating MyViewController(), init method in our viewController is not calling!

we can not use such initializer:

init()
    {
        let displayNode = ASDisplayNode()
        displayNode.backgroundColor = .backgroundColor
        super.init(node: displayNode)
        setup()
    }

@haashem
Copy link

haashem commented Aug 19, 2018

@maicki can you please explain why you have changed the designated initializer? now in the whole project I should replace MyViewController() with MyViewController(node: nil) and I should remember not to call empty initializer else my custom init won't work!

@maicki
Copy link
Contributor Author

maicki commented Aug 20, 2018

@hashemp206 Hey - what is your use case using an ASViewController subclass without initializing it via a node? ... Instead you can just use a plain UIViewController in this case. It's kind of a side effect that ASViewController works as an UIViewController if you initializer it via a nil node. In fact the initWithNode: initializer is declared as assume nonnull, so it should not be even possible.

@haashem
Copy link

haashem commented Aug 21, 2018

@maicki I'm encapsulating node creation in init method
like this:

// MARK: Object lifecycle
    override init(node: ASDisplayNode? = nil)
    {
        let displayNode = ASDisplayNode()
        displayNode.backgroundColor = .backgroundColor
        
        super.init(node: displayNode)
        displayNode.addSubnode(collectionNode)
        hidesBottomBarWhenPushed = true
        setup()
    }

I prefer initiation with empty parameter than passing nil node, because I may forgot to pass nil node, and the init method won't called! I don't create node outside of ViewController and pass it to the ViewController. display node creation is part of ViewController responsibility.

@maicki maicki deleted the MSASViewControllerDesignatedInitializer branch September 8, 2018 16:27
mikezucc pushed a commit to mikezucc/Texture that referenced this pull request Oct 2, 2018
…ureGroup#1054)

* Add NS_DESIGNATED_INITIALIZER to ASViewController initWithNode:

* Add changelog
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