Kira
Kira

Reputation: 1603

Why passing data with delegate fails in Swift 4

This is my protocol

protocol PassDataDelegate {
    func passData(data: String)
}

My first controller

class FirstViewController: UIViewController {

    @IBOutlet weak var textField: UITextField!

    var delegate: PassDataDelegate?

    override func viewDidLoad() {
        super.viewDidLoad()
        delegate = SecondViewController()
    }

    @IBAction func sendDataButtonTapped(_ sender: Any) {
        delegate?.passData(data: textField.text!)
        performSegue(withIdentifier: "Go", sender: nil)
    }

}

And second, final one

class SecondViewController: UIViewController, PassDataDelegate {

    @IBOutlet weak var myLabel: UILabel!

    override func viewDidLoad() {
        super.viewDidLoad()
    }

    func passData(data: String) {
        print("This came from first: \(data). Will change UI.")
        myLabel.text = data
    }
}

App is crashing on label changing part. It says nil while unwrapping optional. What is wrong here?

Upvotes: 0

Views: 1478

Answers (2)

CodingMeSwiftly
CodingMeSwiftly

Reputation: 3261

There are several fundamental issues with your code. I think there might also be some misapprehensions on your side regarding delegation and UIStoryboardSegue mechanism. You should probably read up on that here (Delegation) and here (Segues).

That being said, let me post a solution to your problem with inline comments explaining what's going on.

//  Has to be marked as a class protocol (`: class`) so that
//  `weak var delegate: PassDataDelegate?` can be weak.
protocol PassDataDelegate: class {
  func passData(data: String)
}

class FirstViewController: UIViewController {

  @IBOutlet weak var textField: UITextField!

  //  Important!
  //  Make this a `weak` var. In your case, you would fortunately not create a retain cycle
  //  but there is a big threat of creating those when using delegation patterns with non-weak delegates.
  //
  //  In your case, if you don't make this `weak`, `SecondViewController` would never be deallocated unless you
  //  cleared this var manually (setting it to `nil`).
  //
  //  Also note that, if you're using `PassDataDelegate` solely for forwarding some data to the next view controller,
  //  you can dismiss this var entirely. There is no need to have a reference to the second view controller hanging around.
  //  In fact, as mentioned above, it can be dangerous to do so.
  //  Additionally, you don't need to make the protocol `: class` then.
  private weak var delegate: PassDataDelegate?

  override func viewDidLoad() {
    super.viewDidLoad()

    //  It doesn't make any sense to create a `SecondViewController` here.
    //  The segue mechanism will create a new instance of `SecondViewController`.
//    delegate = SecondViewController()
  }

  @IBAction func sendDataButtonTapped(_ sender: Any) {
    //  `delegate?` is `nil` here.
//    delegate?.passData(data: textField.text!)
    performSegue(withIdentifier: "Go", sender: nil)
  }

  //  This is the proper 'hook' for you to forward data or do anything with a destination
  //  view controller presented using `UIStoryboardSegue`.
  //  This method will be called by the system following your call to `performSegue`.
  override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
    super.prepare(for: segue, sender: sender)

    //  `UITextField.text` can be `nil`, so safeguard for that here.
    //  If the destination implements our protocol, we can forward data to it.
    if let text = textField.text, let delegate = segue.destination as? PassDataDelegate {

      //  This is optional. You can hang on to the destination view controller here, but
      //  review the comments above to reason about whether this makes sense or not.
      self.delegate = delegate

      //  We can safely forward the data (text) here.
      delegate.passData(data: text)
    }
  }
}

SecondViewController can stay as is.


Update

Regarding Delegation

The delegation pattern usually describes a back pointer which talks back to an initiating instance. E.g. using UITableViewDataSource, a UITableView talks back to a thing implementing this protocol to get information about its data and so on.
You are essentially doing the opposite here by forwarding data to SecondViewController. As mentioned in the comments, this code even breaks, because the implementation of passData in SecondViewController is using outlets not yet initialised.

Now you can do one of three things here:

1

Keep the pattern you are using right now (which is not delegation to be precise) and change SecondViewController to make things work

class SecondViewController: UIViewController, PassDataDelegate {

  @IBOutlet weak var myLabel: UILabel!

  private var data: String?

  override func viewDidLoad() {
    super.viewDidLoad()

    //  It is safe to access `myLabel` in `viewDidLoad`. Outlets have been connected.
    if let data = data {
      myLabel.text = data
    }
  }


  func passData(data: String) {
    self.data = data

    //  Only access `myLabel` if the view is loaded.
    if isViewLoaded {
      print("This came from first: \(data). Will change UI.")
      myLabel.text = data
    }
  }
}

This approach is very cumbersome actually, because you need to manoeuvre around the fact that passData may be called at any time. So you don't know if your outlets have been initialised yet, which leads to bloated and repetitive code. Bad.

2

Strip protocols entirely and use a more straightforward approach

class FirstViewController: UIViewController {

  @IBOutlet weak var textField: UITextField!

  //  This is the proper 'hook' for you to forward data or do anything with a destination
  //  view controller presented using `UIStoryboardSegue`.
  //  This method will be called by the system following your call to `performSegue`.
  override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
    super.prepare(for: segue, sender: sender)

    //  `UITextField.text` can be `nil`, so safeguard for that here.
    //  If the destination is a `SecondViewController`, we know that is has `public var data: String` and we can forward data to it.
    if let text = textField.text, let destination = segue.destination as? SecondViewController {

      //  We can safely forward the data (text) here.
      destination.data = text
    }
  }
}

class SecondViewController: UIViewController {

  @IBOutlet weak var myLabel: UILabel!

  //  Deliberatly marking this a `public` to make clear that
  //  you're intented to set this from the 'outside'.
  public var data: String? {
    didSet {
      if isViewLoaded {
        myLabel.text = data
      }
    }
  }

  override func viewDidLoad() {
    super.viewDidLoad()

    //  It is safe to access `myLabel` in `viewDidLoad`. Outlets have been connected.
    if let data = data {
      myLabel.text = data
    }
  }
}

Again, there are things we don't like about his approach:

  • Still repeating code and having to check for isViewLoaded
  • You specifically wanted to use protocols, we don't do that here

We could work around the repetitive code issue by providing the data in an init of SecondViewController. However, since you're using segues, the storyboard will instantiate the destination view controller for you and you cannot gain control over that. Now you could strip using segues, but this quickly moves far away from your original question and is a totally different code only approach. So this is no good either.

3

Use protocols but apply the delegation pattern correctly.

protocol DataProvider: class {
  func provideData() -> String?
}

protocol DataHandler: class {
  var providerDelegate: DataProvider? { get set }
}

class FirstViewController: UIViewController, DataProvider {

  @IBOutlet weak var textField: UITextField!

  func provideData() -> String? {
    return textField.text
  }

  override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
    super.prepare(for: segue, sender: sender)

    //  If the destination is a `DataHandler`, we can set yourselves as its provider.
    if let destination = segue.destination as? DataHandler {
      destination.providerDelegate = self
    }
  }
}

class SecondViewController: UIViewController, DataHandler {

  @IBOutlet weak var myLabel: UILabel!

  weak var providerDelegate: DataProvider?

  override func viewDidLoad() {
    super.viewDidLoad()

    if let data = providerDelegate?.provideData() {
      //  Safe to access `myLabel`, because we are in `viewDidLoad`.
      myLabel.text = data
    }
  }
}

This approach is the most generic. Both parties don't care what exactly the handler and provider are. Note that in a classical delegation pattern, you would probably not have the DataHandler protocol and check for a concrete type (here SecondViewController) in prepareForSegue. However, this approach is more flexible while still having the delegation weaved into it. This approach is also the most robust from the SecondViewController point of view. Instead of having to handle passData at any moment, it can decide itself when to ask its delegate (DataProvider) for the data. This way, SecondViewController can reason about when all of its outlets, etc. are initialised and it is safe to process the data.

Upvotes: 1

vadian
vadian

Reputation: 285082

SecondViewController() is not the controller designed in the storyboard. It's a brand new instance without connected outlets (which is the reason of the crash). You need the real reference to the SecondViewController instance.

Assuming the SecondViewController instance is the destination view controller of the segue you don't need protocol / delegate, pass the data through the segue

class FirstViewController: UIViewController {

    @IBOutlet weak var textField: UITextField!

    @IBAction func sendDataButtonTapped(_ sender: Any) {
        performSegue(withIdentifier: "Go", sender: nil)
    }

    func prepare(for segue: UIStoryboardSegue, sender: Any?) {
        if segue.identifier == "Go" {
           let secondController = segue.destination as! SecondViewController 
           controller.passedData = textField.text!
        }
    }
}

class SecondViewController: UIViewController, PassDataDelegate {

    @IBOutlet weak var myLabel: UILabel!

    var passedData = ""

    override func viewDidLoad() {
        super.viewDidLoad()
        print("This came from first: \(passedData). Will change UI.")
        myLabel.text = passedData
    }
}

Upvotes: 2

Related Questions