UIKitの表示変化をメンバ変数に頼り過ぎてはいけない

はじめに

先日AR Cake Dividerをリリースしましたが、早速バグが見つかりました（汗）

設定画面では設定項目をタップすると、ダイアログを表示し、そこに表示されるPickerで値を選択できる様にしています。そして、設定値は「ガイド形式」と「分割数」と二つ用意しているのですが、交互に表示していると急にアプリが落ちてしまうことがあります。

このバグは私のとりあえずメンバ変数でいいやという安易な考えが招いており、戒めのためにも不具合の原因を説明させて頂きます。

原因

Pickerが表示するリストはそれぞれ別の配列で定義しており、選ばれた設定項目とPickerで選択されたrowの値はViewControllerのメンバ変数として保持しています。

交互にダイアログを操作されるとrowの値はその都度変わりますが、rowの値を元に配列の値を参照しようとしたときに、rowの値が既に書き換えられてしまっており、配列の範囲外の場所を参照してしまいエラーが発生していました。

例えば、上の様に2つのボタンでそれぞれ、別のPickerを表示させ、選択した値でラベルを更新するプログラムがあったとします。

import UIKit

class ViewController: UIViewController, UIPickerViewDelegate, UIPickerViewDataSource {

    @IBOutlet weak var label: UILabel!
    
    enum ButtonId {
        case one
        case two
    }
    
    var selectedButton = ButtonId.one
    var selectedRow = 0
    
    let button1List : [String] = ["apple", "orange", "banana", "grape", "strawberry"]
    let button2List : [String] = ["black", "white"]
    
    override func viewDidLoad() {
        super.viewDidLoad()
        // Do any additional setup after loading the view, typically from a nib.
    }

    @IBAction func pushButton1(_ sender: Any) {
        selectedButton = .one
        showAlert()
    }
    
    @IBAction func pushButton2(_ sender: Any) {
        selectedButton = .two
        showAlert()
    }
    
    func update() {
        switch selectedButton {
        case .one:
            label.text = button1List[selectedRow]
        case .two:
            label.text = button2List[selectedRow]
        default:
            break
        }
    }
    
    private func showAlert() {
        let vc = UIViewController()
        vc.preferredContentSize = CGSize(width: 250,height: 250)
        let pickerView = UIPickerView(frame: CGRect(x: 0, y: 0, width: 250, height: 250))
        pickerView.delegate = self
        pickerView.dataSource = self
        vc.view.addSubview(pickerView)
        
        var title = ""
        switch selectedButton {
        case .one:
            title = "fruit"
        case .two:
            title = "color"
        default:
            break
        }
        
        let editAlert = UIAlertController(title: title, message: "", preferredStyle: UIAlertController.Style.alert)
        editAlert.setValue(vc, forKey: "contentViewController")
        editAlert.addAction(UIAlertAction(title: "done", style: .default, handler: { (UIAlertAction) in
            self.selectedRow = pickerView.selectedRow(inComponent: 0)
            self.update()
        }))
        editAlert.addAction(UIAlertAction(title: "cancel", style: .cancel, handler: nil))
        self.present(editAlert, animated: true)
    }
    
    func numberOfComponents(in pickerView: UIPickerView) -> Int {
        return 1
    }
    
    func pickerView(_ pickerView: UIPickerView, numberOfRowsInComponent component: Int) -> Int {
        switch selectedButton {
        case .one:
            return button1List.count
        case .two:
            return button2List.count
        default:
            return 0
        }
    }
    
    func pickerView(_ pickerView: UIPickerView, titleForRow row: Int, forComponent component: Int) -> String? {
        switch selectedButton {
        case .one:
            return button1List[row]
        case .two:
            return button2List[row]
        default:
            return nil
        }
    }
}

このソースの様に、メンバ変数のselectedRowでPickerの橋渡しを行い、別途update()を呼ぶ様にすると、update()内で配列を参照するタイミングでユーザ操作によりselectedRowの値が書き換えられるとアウトになります。

この様な事を防ぐためにはCで言う"値渡し"でselectedRowの値をupdate()に伝える様にします。

    func update(_ selectedRow : Int) {
        switch selectedButton {
        case .one:
            label.text = button1List[selectedRow]
        case .two:
            label.text = button2List[selectedRow]
        default:
            break
        }
    }
    
    private func showAlert() {
        let vc = UIViewController()
        vc.preferredContentSize = CGSize(width: 250,height: 250)
        let pickerView = UIPickerView(frame: CGRect(x: 0, y: 0, width: 250, height: 250))
        pickerView.delegate = self
        pickerView.dataSource = self
        vc.view.addSubview(pickerView)
        
        var title = ""
        switch selectedButton {
        case .one:
            title = "fruit"
        case .two:
            title = "color"
        default:
            break
        }
        
        let editAlert = UIAlertController(title: title, message: "", preferredStyle: UIAlertController.Style.alert)
        editAlert.setValue(vc, forKey: "contentViewController")
        editAlert.addAction(UIAlertAction(title: "done", style: .default, handler: { (UIAlertAction) in
            let selectedRow = pickerView.selectedRow(inComponent: 0)
            self.update(selectedRow)
        }))
        editAlert.addAction(UIAlertAction(title: "cancel", style: .cancel, handler: nil))
        self.present(editAlert, animated: true)
    }

ついついクラス内でしか使わない値なのだからメンバ変数でいいやという発想になってしまいがちだったので、良い教訓になりました。