Bronze Challenge: Solution + Feedback about Coding Style

iOS Programming (7th Edition) 5. Programmatic Views
1

Here is my solution for the bronze challenge in chapter 5. It wasn’t very easy to figure out but after completing it, I feel a bit more confident in how constraints work under the hood.

A question regarding code style in Swift that I was curious about, in the MapViewController below, I found it to be easier to read and more straight forward to declare the UI elements used on this view controller to be part of the class instead of being defined inside loadView() which is how the book defines them. Is there anything particularly wrong about one way versus the other? Is it just based on preference? What would be considered best practice?

Would love to hear feedback!

class MapViewController: UIViewController {
    var mapView: MKMapView!
    let poiLabel = UILabel()
    let poiSwitch = UISwitch()
    let segmentedControl = UISegmentedControl(items: ["Standard", "Hybrid", "Satellite"])
    
    override func loadView() {
        mapView = MKMapView()

        view = mapView

        segmentedControl.backgroundColor = UIColor.systemBackground
        segmentedControl.selectedSegmentIndex = 0
        segmentedControl.addTarget(self, action: #selector(mapTypeChanged(_:)), for: .valueChanged)

        segmentedControl.translatesAutoresizingMaskIntoConstraints = false
        view.addSubview(segmentedControl)

        poiLabel.text = "Points of Interest"
        poiLabel.translatesAutoresizingMaskIntoConstraints = false

        view.addSubview(poiLabel)

        poiSwitch.translatesAutoresizingMaskIntoConstraints = false
        poiSwitch.isOn = false
        poiSwitch.addTarget(self, action: #selector(poiSelect(_:)), for: .valueChanged)
        
        view.addSubview(poiSwitch)
        
        NSLayoutConstraint.activate([
            segmentedControl.topAnchor.constraint(equalTo: view.safeAreaLayoutGuide.topAnchor, constant: 1),
            segmentedControl.leadingAnchor.constraint(equalTo: view.layoutMarginsGuide.leadingAnchor),
            segmentedControl.trailingAnchor.constraint(equalTo: view.layoutMarginsGuide.trailingAnchor),
            poiLabel.topAnchor.constraint(equalTo: segmentedControl.bottomAnchor, constant: 8),
            poiLabel.leadingAnchor.constraint(equalTo: view.layoutMarginsGuide.leadingAnchor),
            poiSwitch.topAnchor.constraint(equalTo: segmentedControl.bottomAnchor, constant: 8),
            poiSwitch.leadingAnchor.constraint(equalTo: poiLabel.trailingAnchor, constant: 4),
            poiSwitch.centerYAnchor.constraint(equalTo: poiLabel.centerYAnchor),
        ])
    }
    
    override func viewDidLoad() {
        super.viewDidLoad()
    }
    
    @IBAction func poiSelect(_ switchVal: UISwitch) {
        if (switchVal.isOn) {
            mapView.pointOfInterestFilter = MKPointOfInterestFilter.includingAll
        } else {
            mapView.pointOfInterestFilter = MKPointOfInterestFilter.excludingAll
        }
    }
    
    @objc func mapTypeChanged(_ segControl: UISegmentedControl) {
        switch segControl.selectedSegmentIndex {
        case 0:
            mapView.mapType = .standard
            poiLabel.textColor = UIColor.black
        case 1:
            mapView.mapType = .hybrid
            poiLabel.textColor = UIColor.white
        case 2:
            mapView.mapType = .satellite
            poiLabel.textColor = UIColor.white
        default:
            break;
        }
    }
}
2

In general, you want variables to have the smallest scope possible. By limiting access to variables, you reduce the chances for bugs in your program.

If the UI elements are only used within loadView, then they should be declared in loadView. If you need to access them from other functions in MapViewController, or from other classes in the program, then make them class properties.

1 Like
3

Makes sense. Thank you!