Swiftコードのリファクタリングの考え方が分かりません

Question

プログラミング初心者です。
自分でも理解できていない部分が多いので、質問が分かりづらかったらすいません。

SwiftでmacOSアプリを開発しています。

Swiftコードのリファクタリングをしたいのですが、考え方ややり方が分からなくて困っています。
できれば考え方を教えていただきたいと思っています。

    //パラメーター1
    @IBOutlet weak var samp: NSTextField!
    @IBAction func sampling(_ sender: NSSlider) {
        samp.stringValue = String(format: "%.0f",sender.doubleValue)
    }
    @IBOutlet weak var sampling: NSSlider!
    @IBAction func samp(_ sender: Any) {
        sampling.stringValue = String((sender as AnyObject).doubleValue)
    }
    //パラメーター2
    @IBOutlet weak var flame: NSTextField!
    @IBAction func flamePriod(_ sender: NSSlider) {
        flame.stringValue = String(format: "%.0f",sender.doubleValue)
    }
    @IBOutlet weak var flamePriod: NSSlider!
    @IBAction func flame(_ sender: Any) {
        flamePriod.stringValue = String((sender as AnyObject).doubleValue)
    }
    //パラメーター3
    @IBOutlet weak var alp: NSTextField!
    @IBAction func allPass(_ sender: NSSlider) {
        alp.stringValue = String(format: "%.2f",sender.doubleValue)
    }
    @IBOutlet weak var allPass: NSSlider!
    @IBAction func alp(_ sender: Any) {
        allPass.stringValue = String((sender as AnyObject).doubleValue)
    }

    //ボタン1
    @IBAction func defaultA(_ sender: Any) {
        samp.stringValue = "38000"
        sampling.stringValue = "38000"
        flame.stringValue = "300"
        flamePriod.stringValue = "300"
        alp.stringValue = "0.5"
        allPass.stringValue = "0.5"
    }
    
    //ボタン2
    @IBAction func defaultB(_ sender: Any) {
        samp.stringValue = "40000"
        sampling.stringValue = "40000"
        flame.stringValue = "200"
        flamePriod.stringValue = "200"
        alp.stringValue = "0.6"
        allPass.stringValue = "0.6"
    }
    
    //ボタン3
    @IBAction func defaultC(_ sender: Any) {
        samp.stringValue = "48000"
        sampling.stringValue = "48000"
        flame.stringValue = "240"
        flamePriod.stringValue = "240"
        alp.stringValue = "0.55"
        allPass.stringValue = "0.55"
    }

やりたいこと①：IBOutletやIBactionをまとめたい
iOSではOutletcorrectionというものがあり、Outlet接続はまとめることができると検索によりわかったのですが、macOSでもそれに準ずるやり方はないのでしょうか？

やりたいこと②：ボタン１〜３をリファクタリングしたい
ボタン1〜3には上記のように定数を代入するのですが、これをきれいにまとめることはできないでしょうか？
実際にはパラメーターが10個ほどあるので、配列や関数を使ってリファクタリングしたいと思っているのですが、考え方が分からなくて困っています。。

Answer 1

macOSに関してはよく知らない&Storyboardをみていないので軽めの回答になりますが、

1. リーダブルコードという本を読む
2. 命名を見直す
3. パラメーター1〜3を抽象化したcustomViewを作る
4. ボタン1〜3内のプロパティ代入を抽象化した関数を作る
5. IBAction内のメソッドは可能な限り1行に纏める
6. enumに定数を纏める

くらいは自分ならやります。
多分このコードが書かれてる場所ってViewController内だと思うのですがあまりにコード量が大きくなりすぎたら別クラスを作って分けたりもします。

リファクタリングに関して、というわけではありませんが素晴らしい記事があったのでリンクも貼っておきます。個人的には一番のお気に入りで何度も見返してます。

https://academy.realm.io/jp/posts/tryswift-daniel-steinberg-blending-cultures/

googleで「文化を調和させる swift」で調べるとこの記事を実践された方のコードがgithubにも上がってます。そちらも合わせて読むと分かりやすいですよ。

###追記

誰か何か回答してくれるんじゃないかと期待しながらちょくちょく見てたのですがあまり反応がないのでこっそり投稿です。とは言ってもmacOSは上記の通り門外漢ですのでiOSで同じような構成をとってます。またよく分からなかったものに関しては置き換え等々しています（senderの型のAnyとか）。なお命名に関してはかなりザル管理です。ここもっと良く出来るんじゃね？みたいな部分がありましたらご教授いただけましたら嬉しいです（とっても）。もし参考になれば幸いです。

//(6)
enum containerType: Int {
    case top = 0
    case middle = 1
    case bottom = 2
}

enum propertyType {
    case typeA
    case typeB
    case typeC
}

enum properties {
    
    static let defaultA: [String] = ["38000", "300", "0.5"]
    static let defaultB: [String] = ["40000", "200", "0.6"]
    static let defaultC: [String] = ["48000", "240", "0.55"]
    
    static func pickupPropertiesSet(_ property: propertyType) -> (containerType) -> String {
        switch property {    
        case .typeA:
            return { (controller: containerType) -> String in
                return defaultA[controller.rawValue]
            }
        case .typeB:
            return { (controller: containerType) -> String in
                return defaultB[controller.rawValue]
            }
        case .typeC:
            return { (controller: containerType) -> String in
                return defaultC[controller.rawValue]
            }
        }   
    }
}

class ViewController: UIViewController {
    
    var myFunctions = Functions()

    //IBOutletCollection 三つ程度なら普段ならまとめたりしないです。
    @IBOutlet var containers: [ParameterContainerView]!

    //(5)
    @IBAction func tapDefaultA(_ sender: UIButton) {
        myFunctions.defaultProperties(typeOf: .typeA, containers: containers)
    }
    @IBAction func tapDefaultB(_ sender: UIButton) {
        myFunctions.defaultProperties(typeOf: .typeB, containers: containers)
    }
    @IBAction func tapDefaultC(_ sender: UIButton) {
        myFunctions.defaultProperties(typeOf: .typeC, containers: containers)
    }
    
    override func viewDidLoad() {
        super.viewDidLoad()
    }

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

}

//(3)
class ParameterView: UIView {

    @IBOutlet weak var firstTextField: UITextField!
    @IBAction func actionFirstTextField(_ sender: UISlider) {
        firstTextField.text = String(sender.value)
    }
    
    @IBOutlet weak var secondTextField: UITextField!
    @IBAction func actionSecondTextField(_ sender: UISlider) {
        secondTextField.text = String(sender.value)
    }
    
    func fill(with text: String) {
        firstTextField.text = text
        secondTextField.text = text
    }

}

//(3)
class ParameterContainerView: UIView {

    override init(frame: CGRect) {
        super.init(frame: frame)
        load()
        
    }
    
    required init?(coder aDecoder: NSCoder) {
        super.init(coder: aDecoder)
        load()
    }
    
    func load() {
        if self.subviews.isEmpty {
            let testView = ParameterView.instantiate()
            testView.frame = self.bounds
            self.addSubview(testView)
        }
    }
    
    open func accessParameterView() -> ParameterView {
        guard let view = self.subviews.first as? ParameterView else {
            fatalError()
        }
        return view
    }
    
    func fill(with text: String) {
        let parameterView = accessParameterView()
        parameterView.fill(with: text)
    }

}

extension NSObject {
    class var className: String {
        return String(describing: self)
    }
    
    var className: String {
        return type(of: self).className
    }
}

protocol NibInstantiatable {}
extension UIView: NibInstantiatable {}

extension NibInstantiatable where Self: UIView {
    static func instantiate(withOwner ownerOrNil: Any? = nil) -> Self {
        let nib = UINib(nibName: self.className, bundle: nil)
        return nib.instantiate(withOwner: ownerOrNil, options: nil)[0] as! Self
    }
}

class Functions {
    
    private func callDefaultProperty(_ propertyType: propertyType) -> ((containerType) -> String) {
        return properties.pickupPropertiesSet(propertyType)
    }
    //(4)
    func defaultProperties(typeOf type: propertyType, containers: [ParameterContainerView]) {
        containers.enumerated().forEach { (index, container) in
            let text = callDefaultProperty(type)(containerType(rawValue: index)!)
            container.fill(with: text)
        }
    }
    
    
}

Comments

[besco]

ありがとうございます！
ヒントを頂けただけでも助かります。

今の私のレベルでは難しい内容ですが、アドバイス内容を調べて頑張って自分でできるようにします。

[besco]

xAxisさん、本当にありがとうございました。
まだプログラミングを始めて日も短く勉強不足なため、折角書いていただいたコードをまだ理解しきれていないのが申し訳ないです(汗)
理解できそうな部分を参考にさせていただきつつ、以下のように書き直しました。

```swift
//Outletのパラメーターを纏めた
@IBOutlet weak var samp, flame, alp: NSTextField!
@IBOutlet weak var sampling, flamePriod, allPass : NSSlider!

//パラメーター1
@IBAction func sampling(_ sender: NSSlider) {
samp.stringValue = String(format: "%.0f",sender.doubleValue)
}
@IBAction func samp(_ sender: Any) {
sampling.stringValue = String((sender as AnyObject).doubleValue)
}
//パラメーター2
@IBAction func flamePriod(_ sender: NSSlider) {
flame.stringValue = String((sender as AnyObject).doubleValue)
}
@IBAction func flame(_ sender: Any) {
flamePriod.stringValue = String((sender as AnyObject).doubleValue)
}
//パラメーター3
@IBAction func allPass(_ sender: NSSlider) {
alp.stringValue = String(format: "%.2f",sender.doubleValue)
}
@IBAction func alp(_ sender: Any) {
allPass.stringValue = String((sender as AnyObject).doubleValue)
}

//ボタンのパラメーター設定を辞書型で記述
@IBAction func defaultButton(_ sender: NSButton) {
var defaultparam:[String:[String:String]] = [
"ボタン1" : ["samp": "38000", "flame": "300","alp": "0.5"],
"ボタン2" : ["samp": "40000", "flame": "200","alp": "0.6"],
"ボタン3" : ["samp": "48000", "flame": "240","alp": "0.55"]
]
//switch文でボタン管理
switch sender.title {
case "ボタン1":
samp.stringValue = defaultparam["ボタン1"]!["samp"]!
sampling.stringValue = defaultparam["ボタン1"]!["samp"]!
flame.stringValue = defaultparam["ボタン1"]!["flame"]!
flamePriod.stringValue = defaultparam["ボタン1"]!["flame"]!
alp.stringValue = defaultparam["ボタン1"]!["alp"]!
allPass.stringValue = defaultparam["ボタン1"]!["alp"]!
case "ボタン2":
samp.stringValue = defaultparam["ボタン2"]!["samp"]!
sampling.stringValue = defaultparam["ボタン2"]!["samp"]!
flame.stringValue = defaultparam["ボタン2"]!["flame"]!
flamePriod.stringValue = defaultparam["ボタン2"]!["flame"]!
alp.stringValue = defaultparam["ボタン2"]!["alp"]!
allPass.stringValue = defaultparam["ボタン2"]!["alp"]!
case "ボタン3":
samp.stringValue = defaultparam["ボタン3"]!["samp"]!
sampling.stringValue = defaultparam["ボタン3"]!["samp"]!
flame.stringValue = defaultparam["ボタン3"]!["flame"]!
flamePriod.stringValue = defaultparam["ボタン3"]!["flame"]!
alp.stringValue = defaultparam["ボタン3"]!["alp"]!
allPass.stringValue = defaultparam["ボタン3"]!["alp"]!
default :
break
}
}
```

[xAxis]

最初に書かれたコードよりも良くなってますよ。全体を俯瞰した時前よりもテンポよく読めます。また気になる点がありましたら遠慮なくどうぞ。

[besco]

xAxisさん！ご親切にありがとうございました！
アドバイス頂いたことをまだ理解できていない部分も多いので、とりあえずはアドバイスいただいた用語や意味を調べつつ、コードを見直すという形でリファクタリングしてみようと思います。
わかりづらい質問に丁寧に回答頂きましてありがとうございました。
とても助かりました。