passing data between vcs using delegate and protocol #13
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="3.0" toolsVersion="14490.70" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" useSafeAreas="YES" colorMatched="YES"> | ||
| <device id="retina6_1" orientation="portrait"> | ||
| <adaptation id="fullscreen"/> | ||
| </device> | ||
| <dependencies> | ||
| <deployment identifier="iOS"/> | ||
| <plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="14490.49"/> | ||
| <capability name="Named colors" minToolsVersion="9.0"/> | ||
| <capability name="Safe area layout guides" minToolsVersion="9.0"/> | ||
| <capability name="documents saved in the Xcode 8 format" minToolsVersion="8.0"/> | ||
| </dependencies> | ||
| <scenes> | ||
| <!--Jimmy View Controller--> | ||
| <scene sceneID="i6n-Jf-cUm"> | ||
| <objects> | ||
| <viewController storyboardIdentifier="JimmyViewController" id="JQL-P8-rL1" customClass="JimmyViewController" customModule="BackToBasicsUIKit" customModuleProvider="target" sceneMemberID="viewController"> | ||
| <view key="view" contentMode="scaleToFill" id="2bV-vk-ILW"> | ||
| <rect key="frame" x="0.0" y="0.0" width="414" height="896"/> | ||
| <autoresizingMask key="autoresizingMask" widthSizable="YES" heightSizable="YES"/> | ||
| <subviews> | ||
| <button opaque="NO" contentMode="scaleToFill" contentHorizontalAlignment="center" contentVerticalAlignment="center" buttonType="roundedRect" lineBreakMode="middleTruncation" translatesAutoresizingMaskIntoConstraints="NO" id="9xb-bj-TfT"> | ||
| <rect key="frame" x="0.0" y="424" width="414" height="48"/> | ||
| <color key="backgroundColor" white="0.33333333333333331" alpha="1" colorSpace="custom" customColorSpace="genericGamma22GrayColorSpace"/> | ||
| <constraints> | ||
| <constraint firstAttribute="height" constant="48" id="1hd-GI-YcB"/> | ||
| </constraints> | ||
| <fontDescription key="fontDescription" type="system" pointSize="30"/> | ||
| <state key="normal" title="Beam me up Scotty!"> | ||
| <color key="titleColor" white="1" alpha="1" colorSpace="custom" customColorSpace="genericGamma22GrayColorSpace"/> | ||
| </state> | ||
| <connections> | ||
| <action selector="buttonPressed:" destination="JQL-P8-rL1" eventType="touchUpInside" id="bwV-ls-74l"/> | ||
| </connections> | ||
| </button> | ||
| <textField opaque="NO" contentMode="scaleToFill" contentHorizontalAlignment="left" contentVerticalAlignment="center" borderStyle="roundedRect" placeholder="type something to beam up!" textAlignment="center" minimumFontSize="17" translatesAutoresizingMaskIntoConstraints="NO" id="Id6-G3-T4c"> | ||
| <rect key="frame" x="20" y="314" width="374" height="40"/> | ||
| <constraints> | ||
| <constraint firstAttribute="height" constant="40" id="9zj-IF-OEb"/> | ||
| </constraints> | ||
| <nil key="textColor"/> | ||
| <fontDescription key="fontDescription" type="system" pointSize="25"/> | ||
| <textInputTraits key="textInputTraits"/> | ||
| </textField> | ||
| <label opaque="NO" userInteractionEnabled="NO" contentMode="left" horizontalHuggingPriority="251" verticalHuggingPriority="251" text="Label" textAlignment="natural" lineBreakMode="tailTruncation" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" translatesAutoresizingMaskIntoConstraints="NO" id="ASx-aW-Sso"> | ||
| <rect key="frame" x="160" y="226" width="94" height="48"/> | ||
| <fontDescription key="fontDescription" type="system" pointSize="40"/> | ||
| <nil key="textColor"/> | ||
| <nil key="highlightedColor"/> | ||
| </label> | ||
| </subviews> | ||
| <color key="backgroundColor" name="blue"/> | ||
| <constraints> | ||
| <constraint firstItem="ASx-aW-Sso" firstAttribute="centerX" secondItem="2bV-vk-ILW" secondAttribute="centerX" id="24y-0o-bVf"/> | ||
| <constraint firstItem="Id6-G3-T4c" firstAttribute="leading" secondItem="tyl-He-E4t" secondAttribute="leading" constant="20" id="2dK-7h-mbb"/> | ||
| <constraint firstItem="9xb-bj-TfT" firstAttribute="centerX" secondItem="2bV-vk-ILW" secondAttribute="centerX" id="8Ke-FD-cph"/> | ||
| <constraint firstItem="tyl-He-E4t" firstAttribute="trailing" secondItem="9xb-bj-TfT" secondAttribute="trailing" id="8RX-IK-yX0"/> | ||
| <constraint firstItem="9xb-bj-TfT" firstAttribute="leading" secondItem="tyl-He-E4t" secondAttribute="leading" id="NUq-0c-a4B"/> | ||
| <constraint firstItem="9xb-bj-TfT" firstAttribute="centerY" secondItem="2bV-vk-ILW" secondAttribute="centerY" id="UYx-u0-vgw"/> | ||
| <constraint firstItem="Id6-G3-T4c" firstAttribute="top" secondItem="ASx-aW-Sso" secondAttribute="bottom" constant="40" id="mXl-m9-Clu"/> | ||
| <constraint firstItem="tyl-He-E4t" firstAttribute="trailing" secondItem="Id6-G3-T4c" secondAttribute="trailing" constant="20" id="siA-cI-S5u"/> | ||
| <constraint firstItem="9xb-bj-TfT" firstAttribute="top" secondItem="Id6-G3-T4c" secondAttribute="bottom" constant="70" id="udW-Cb-MfP"/> | ||
| </constraints> | ||
| <viewLayoutGuide key="safeArea" id="tyl-He-E4t"/> | ||
| </view> | ||
| <connections> | ||
| <outlet property="label" destination="ASx-aW-Sso" id="HZ2-7F-AkD"/> | ||
| <outlet property="textField" destination="Id6-G3-T4c" id="cR4-5e-ySR"/> | ||
| <segue destination="8BQ-iN-CpD" kind="show" identifier="sendDataForwards" id="rFD-NF-eVe"/> | ||
| </connections> | ||
| </viewController> | ||
| <placeholder placeholderIdentifier="IBFirstResponder" id="JNV-LY-xwq" userLabel="First Responder" sceneMemberID="firstResponder"/> | ||
| </objects> | ||
| <point key="canvasLocation" x="-136.23188405797103" y="87.723214285714278"/> | ||
| </scene> | ||
| <!--Jimmy Second View Controller--> | ||
| <scene sceneID="lbN-gP-vOp"> | ||
| <objects> | ||
| <viewController storyboardIdentifier="JimmySecondViewController" id="8BQ-iN-CpD" customClass="JimmySecondViewController" customModule="BackToBasicsUIKit" customModuleProvider="target" sceneMemberID="viewController"> | ||
| <view key="view" contentMode="scaleToFill" id="azh-A8-7oV"> | ||
| <rect key="frame" x="0.0" y="0.0" width="414" height="896"/> | ||
| <autoresizingMask key="autoresizingMask" widthSizable="YES" heightSizable="YES"/> | ||
| <subviews> | ||
| <textField opaque="NO" contentMode="scaleToFill" contentHorizontalAlignment="left" contentVerticalAlignment="center" borderStyle="roundedRect" placeholder="send something back!" textAlignment="center" minimumFontSize="17" translatesAutoresizingMaskIntoConstraints="NO" id="HTX-5E-tPi"> | ||
| <rect key="frame" x="20" y="314" width="374" height="40"/> | ||
| <constraints> | ||
| <constraint firstAttribute="height" constant="40" id="GOe-UA-uFQ"/> | ||
| </constraints> | ||
| <nil key="textColor"/> | ||
| <fontDescription key="fontDescription" type="system" pointSize="25"/> | ||
| <textInputTraits key="textInputTraits"/> | ||
| </textField> | ||
| <label opaque="NO" userInteractionEnabled="NO" contentMode="left" horizontalHuggingPriority="251" verticalHuggingPriority="251" text="Label" textAlignment="natural" lineBreakMode="tailTruncation" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" translatesAutoresizingMaskIntoConstraints="NO" id="sQh-Fq-jF0"> | ||
| <rect key="frame" x="160" y="226" width="94" height="48"/> | ||
| <fontDescription key="fontDescription" type="system" pointSize="40"/> | ||
| <nil key="textColor"/> | ||
| <nil key="highlightedColor"/> | ||
| </label> | ||
| <button opaque="NO" contentMode="scaleToFill" contentHorizontalAlignment="center" contentVerticalAlignment="center" buttonType="roundedRect" lineBreakMode="middleTruncation" translatesAutoresizingMaskIntoConstraints="NO" id="cM5-3S-kGk"> | ||
| <rect key="frame" x="0.0" y="424" width="414" height="48"/> | ||
| <color key="backgroundColor" white="0.33333333329999998" alpha="1" colorSpace="custom" customColorSpace="genericGamma22GrayColorSpace"/> | ||
| <constraints> | ||
| <constraint firstAttribute="height" constant="48" id="BTx-hH-HGq"/> | ||
| </constraints> | ||
| <fontDescription key="fontDescription" type="system" pointSize="30"/> | ||
| <state key="normal" title="Pass it back!"> | ||
| <color key="titleColor" white="1" alpha="1" colorSpace="custom" customColorSpace="genericGamma22GrayColorSpace"/> | ||
| </state> | ||
| <connections> | ||
| <action selector="buttonPressed:" destination="JQL-P8-rL1" eventType="touchUpInside" id="Sck-QX-Ebq"/> | ||
| <action selector="passItBack:" destination="8BQ-iN-CpD" eventType="touchUpInside" id="EJW-bK-np7"/> | ||
| </connections> | ||
| </button> | ||
| </subviews> | ||
| <color key="backgroundColor" name="purple"/> | ||
| <constraints> | ||
| <constraint firstItem="cM5-3S-kGk" firstAttribute="top" secondItem="HTX-5E-tPi" secondAttribute="bottom" constant="70" id="483-i9-3QK"/> | ||
| <constraint firstItem="cM5-3S-kGk" firstAttribute="leading" secondItem="tQo-rm-UpT" secondAttribute="leading" id="5mg-Pd-EAa"/> | ||
| <constraint firstItem="HTX-5E-tPi" firstAttribute="top" secondItem="sQh-Fq-jF0" secondAttribute="bottom" constant="40" id="85I-rH-Cxs"/> | ||
| <constraint firstItem="sQh-Fq-jF0" firstAttribute="centerX" secondItem="azh-A8-7oV" secondAttribute="centerX" id="FSy-Uf-dWc"/> | ||
| <constraint firstItem="cM5-3S-kGk" firstAttribute="centerY" secondItem="azh-A8-7oV" secondAttribute="centerY" id="NgY-Yt-SEY"/> | ||
| <constraint firstItem="tQo-rm-UpT" firstAttribute="trailing" secondItem="cM5-3S-kGk" secondAttribute="trailing" id="Njt-Fa-JAV"/> | ||
| <constraint firstItem="HTX-5E-tPi" firstAttribute="leading" secondItem="tQo-rm-UpT" secondAttribute="leading" constant="20" id="gFh-qQ-kmK"/> | ||
| <constraint firstItem="tQo-rm-UpT" firstAttribute="trailing" secondItem="HTX-5E-tPi" secondAttribute="trailing" constant="20" id="tO7-rY-lAi"/> | ||
| <constraint firstItem="cM5-3S-kGk" firstAttribute="centerX" secondItem="azh-A8-7oV" secondAttribute="centerX" id="zIK-52-VPr"/> | ||
| </constraints> | ||
| <viewLayoutGuide key="safeArea" id="tQo-rm-UpT"/> | ||
| </view> | ||
| <connections> | ||
| <outlet property="label" destination="sQh-Fq-jF0" id="vI3-27-Tua"/> | ||
| <outlet property="textField" destination="HTX-5E-tPi" id="sXC-Wh-Trr"/> | ||
| </connections> | ||
| </viewController> | ||
| <placeholder placeholderIdentifier="IBFirstResponder" id="nFt-Yi-1yd" userLabel="First Responder" sceneMemberID="firstResponder"/> | ||
| </objects> | ||
| <point key="canvasLocation" x="633" y="88"/> | ||
| </scene> | ||
| </scenes> | ||
| <resources> | ||
| <namedColor name="blue"> | ||
| <color red="0.20000000000000001" green="0.73333333333333328" blue="0.93333333333333335" alpha="1" colorSpace="custom" customColorSpace="sRGB"/> | ||
| </namedColor> | ||
| <namedColor name="purple"> | ||
| <color red="0.73333333333333328" green="0.26666666666666666" blue="0.8666666666666667" alpha="1" colorSpace="custom" customColorSpace="sRGB"/> | ||
| </namedColor> | ||
| </resources> | ||
| </document> | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||
| // | ||||||
| // JimmySecondViewController.swift | ||||||
| // BackToBasicsUIKit | ||||||
| // | ||||||
| // Created by Jimmy Gutierrez on 5/23/19. | ||||||
| // Copyright © 2019 SwiftMiami. All rights reserved. | ||||||
| // | ||||||
|
|
||||||
| import UIKit | ||||||
|
|
||||||
| protocol canRecieve { | ||||||
|
Member
There was a problem hiding this comment. Names of protocols, classes, structs, etc should all be capitalised. The name should also be descriptive, and spelled correctly (
Contributor
There was a problem hiding this comment. I would add that even then I find this name wrong. The typical naming convention for something like this is While delegates use protocols they have a specific use case and conventions. When you work in teams following platform and community conventions is super important. |
||||||
| func dataRecieved(data: String) | ||||||
|
Member
There was a problem hiding this comment. The function signature should also include the sender, so the delegate knows who called it:
Contributor
There was a problem hiding this comment. I know that you (Jimmy) were trying to follow the protocol pattern of a capability but I think you should scrap this idea and think of it as a Delegate specific to |
||||||
| } | ||||||
|
|
||||||
| class JimmySecondViewController: UIViewController { | ||||||
|
|
||||||
| var delegate : canRecieve? | ||||||
|
Contributor
There was a problem hiding this comment. Let me introduce you to memory leaks. Let me know if you see why you have memory leak here and how you'd fix it. Also the space should only go after the
Suggested change
|
||||||
|
|
||||||
| var data = "" | ||||||
|
Member
There was a problem hiding this comment. Renaming to
Contributor
There was a problem hiding this comment. Always important for names to make sense.
Contributor
There was a problem hiding this comment. I would also go further and add that using the noun data makes this too generic. This is text we're talking here, it's a very specific type of data. The name should reflect not only in its use case but also its identity. |
||||||
|
|
||||||
| @IBOutlet var label: UILabel! | ||||||
|
Member
There was a problem hiding this comment. IBOutlets should be declared as
Contributor
There was a problem hiding this comment. Private makes sense. @ivancr regarding this, should IBOutlets be weak or strong, and why?
Member
There was a problem hiding this comment. Technically VC is not the owner of the label, the VC's view is and the VC already has a strong reference to its view. If the VC also kept strong references to the view's children (e.g. this label) then those won't be deallocated if the view was to get swapped out. There may be use cases where you'd want a
Contributor
There was a problem hiding this comment. Apple recommends that in the ARC age your IBOutlets should be strong:
Member
There was a problem hiding this comment. @ivancr i watched that session and was curious as to why they said that, as their documentation, Xcode defaults and sample code reflects the opposite. I like this roundup: https://cocoacasts.com/should-outlets-be-weak-or-strong/ TLDR: It doesn't really matter — just be consistent. |
||||||
| @IBOutlet var textField: UITextField! | ||||||
|
|
||||||
| override func viewDidLoad() { | ||||||
| super.viewDidLoad() | ||||||
| label.text = data | ||||||
|
|
||||||
| } | ||||||
|
|
||||||
|
|
||||||
|
Contributor
There was a problem hiding this comment. Please remove all these empty lines. It may sound OCD but being neat with your code is very important. You're sharing this codebase with a team, please clean up after yourself. |
||||||
| @IBAction func passItBack(_ sender: Any) { | ||||||
|
Member
There was a problem hiding this comment. While this pattern works, it assumes that the previous view controller is the same as its delegate. A slightly better pattern may be to send the data back to the delegate and then allow the delegate to control any navigation (in this case popping the VC). However, a much better approach is to use unwind segues here as they handle all of this for you, without the need of a delegate protocol or handling the navigation yourself. This is a great tutorial on this type of segue: https://matteomanferdini.com/unwind-segue/
Contributor
There was a problem hiding this comment. Didn't know that! Awesome!
Contributor
There was a problem hiding this comment. I'm sure there's somebody out there using these and they have their merits but I've never seen or work in any team that used them. I'd highly recommend you go the Delegate pattern. Also, each part of the whole system should be named accordingly. This IBAction maybe happens when you tap a button, then make the name say that. What you do inside is another story for me. |
||||||
| delegate?.dataRecieved(data: textField.text!) | ||||||
|
Member
There was a problem hiding this comment. You should guard that the text field actually contains text, rather than force unwrapping.
Contributor
There was a problem hiding this comment. Indeed, there's very few instances where it does make sense to force unwrap this is not one of them. |
||||||
| self.navigationController?.popViewController(animated: true) | ||||||
|
Contributor
There was a problem hiding this comment. Agreed. You only need to use |
||||||
| // dismiss(animated: true, completion: nil) | ||||||
|
Member
There was a problem hiding this comment. Code that isn't necessary should be removed to avoid confusion.
Contributor
There was a problem hiding this comment. @jimmygtz15 Instead of commenting, can just delete the commented code. |
||||||
|
|
||||||
| } | ||||||
|
|
||||||
|
|
||||||
|
|
||||||
| } | ||||||
|
Contributor
There was a problem hiding this comment. Please delete empty lines too. |
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to add leading/trailing constraints to avoid the text from disappearing off screen if the label contents are very long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Give it some constraints
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or better yet, get rid of the storyboard and do it programmatically. You can pair with @gionoa and he'll show you the ways of the force.