リーダブルコードで読みにくいコードを改善していく ~ 美しさ編

<p>どうも、ねこきち（<a href="https://mobile.twitter.com/nekokichi1_yos2">@nekokichi1_yos2</a>）です。</p>

リーダブルコード改善シリーズ、第3弾、です。

（詳しくは下記をご参照ください）
[リーダブルコードで読みにくいコードを改善していく ~ 名前編]
(https://qiita.com/NEKOKICHI2/items/dc774bd79b1623032712)
[リーダブルコードで読みにくいコードを改善していく ~ 準備編](https://qiita.com/NEKOKICHI2/items/0b7b1de064dd23a725fd)

**今回は、コードを美しいコードに改善していきます。**

## リーダブルコードの教え

・共通する要素を持つコードをまとめる

・見た目の並びを改行やスペースで整列させる
 ->目を1,2方向に動かすだけで各コードを確認できる
 ->ただし、似たコードがあれば、手間が増える

・ブロック、グループに分ける
 ->複数のコードを一括して理解できる

・処理の手順を段落で分ける

・一貫性を持たせる
　->コーディング規約など
　->何人もの個人的な解釈や書き方が混在し、読みにくくなる

## 美しくないコードを探す

### ViewController

```swift
    override func viewWillAppear(_ animated: Bool) {
        super.viewWillAppear(animated)
        let realm = try! Realm()
        attributedTextArray = [NSAttributedString]()
        memoList = realm.objects(MemoModel.self)
        if memoList.count > 0 {
            for i in 0...memoList.count-1 {
                let attributeText = try! NSKeyedUnarchiver.unarchiveTopLevelObjectWithData(memoList![i].data) as! NSAttributedString
                attributedTextArray.append(attributeText)
            }
        }
        memoTableView.reloadData()
    }
```

・空行がないので、全てのコードが重要に見えてしまい、読みづらい
・Realmのデータに関する処理だと分かるが、コメントがあれば、細かく把握できる
・Realmの処理とTableViewの処理は空行で分けた方がいい


```swift
    override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
        if segue.identifier == "display" {
            let vc = segue.destination as! DisplayMemo
            vc.selectedMemoObject = memoList[memoTableView.indexPathForSelectedRow!.row]
            vc.selectedMemo_attributedText = attributedTextArray[memoTableView.indexPathForSelectedRow!.row]
            vc.selectedIndexPathRow = memoTableView.indexPathForSelectedRow?.row
        }
    }
```

・インスタンスの取得、プロパティへの代入、は分けた方が見やすい
・プロパティへの代入が3度行われてるので、縦に揃えれば見やすい

### AddMemo

```swift
    @IBAction func addMemo(_ sender: Any) {
        let realm = try! Realm()
        let memoObject: MemoModel = MemoModel()
        let archivedAttributedText = try! NSKeyedArchiver.archivedData(withRootObject: memoTextView.attributedText!, requiringSecureCoding: false)
        memoObject.data = archivedAttributedText
        memoObject.identifier = String().randomString()
        try! realm.write{
            realm.add(memoObject)
        }
        self.navigationController?.popViewController(animated: true)
    }
```

・変数の宣言、値の代入、書き込み、は段落で分けた方がいい
・Realmの書き込み処理、ナビゲーションコントローラの戻る処理、をクロージャでまとめる方法もある


```swift
    func imagePickerController(_ picker: UIImagePickerController, didFinishPickingMediaWithInfo info: [UIImagePickerController.InfoKey : Any]) {
        if let pickerImage = info[.originalImage] as? UIImage {
            let mutAttrMemoText = NSMutableAttributedString(attributedString: memoTextView.attributedText)
            let resizedImage = pickerImage.resizeImage(withPercentage: 0.1)!
            let width = pickerImage.size.width
            let padding: CGFloat = self.view.frame.width / 2
            let scaleRate = width / (memoTextView.frame.size.width - padding)
            let imageAttachment = NSTextAttachment()
            imageAttachment.image = UIImage(cgImage: resizedImage.cgImage!, scale: scaleRate, orientation: resizedImage.imageOrientation)
            let imageAttributedString = NSAttributedString(attachment: imageAttachment)
            mutAttrMemoText.append(imageAttributedString)
            memoTextView.attributedText = mutAttrMemoText
        }
        dismiss(animated: true, completion: nil)
    }
```

・変数宣言、代入・実行の処理、はそれぞれでまとめるべき
・変数宣言の行が多いので、何らかの順序で並べ方が良い

### DisplayMemo

```swift
    var selectedMemoObject: MemoModel = MemoModel()
    var selectedMemo_attributedText = NSAttributedString()
    var selectedIndexPathRow: Int!
```
・=を縦に揃えた方がいい
・型指定、初期化、どちらで型を決めるかを共通にするべき

### EditMemo

```swift
    let imagePicker = UIImagePickerController()
    var memoList:Results<MemoModel>!
    var selectedMemoObject: MemoModel = MemoModel()
    var selectedMemo_attributedText = NSAttributedString()
    var selectedIndexPathRow: Int!
```

・ジェネリクスの宣言、インスタンスの宣言、変数の宣言、に分けてまとめた方がいい
（扱うデータや存在感においても、変数よりインスタンスやジェネリクスの方が大きい）
・型の書き方は一貫して揃えた方がいい
（型の指定、インスタンスでの初期化、のどちらかにするべき）

```swift
    override func viewDidLoad() {
        super.viewDidLoad()
        imagePicker.delegate = self
        imagePicker.sourceType = .photoLibrary
        let realm = try! Realm()
        memoList = realm.objects(MemoModel.self)
        memoTextView.attributedText = selectedMemo_attributedText
    }
```

・UIImagePickerの処理、値の代入は分けるべき
・Realmのインスタンスは、Realmの処理にしか使用しないので、スコープを広げた方がいい

## 美しいコードに変える


### ViewController

```swift
    override func viewWillAppear(_ animated: Bool) {
        super.viewWillAppear(animated)
        //初期化
        attributedTextArray = [NSAttributedString]()
        //データ取得
        memoList            = realm.objects(MemoModel.self)
        
        //データをNSAttributedStringに変換
        if memoList.count > 0 {
            for i in 0...memoList.count-1 {
                let attributeText = try! NSKeyedUnarchiver.unarchiveTopLevelObjectWithData(memoList![i].data) as! NSAttributedString
                attributedTextArray.append(attributeText)
            }
        }
        
        //tableView更新
        memoTableView.reloadData()
    }
```

・初期化&代入、Realmのデータに関する処理、その他、の段落に分けた
・代入、複雑な処理、メソッドの実行、と区別できるように空行を追加

```swift
    override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
        if segue.identifier == "display" {
            //DisplayMemoのインスタンスを宣言
            let vc = segue.destination as! DisplayMemo
            //DisplayMemoのプロパティに代入
            vc.selectedMemoObject          = memoList[memoTableView.indexPathForSelectedRow!.row]
            vc.selectedMemo_attributedText = attributedTextArray[memoTableView.indexPathForSelectedRow!.row]
            vc.selectedIndexPathRow        = memoTableView.indexPathForSelectedRow?.row
        }
    }
```
・一目で何の処理か理解できるが、中の全コードがDisplayMemoに関する処理なので、区別できるようコメントを追加
・代入のコードが連続してるので、=を縦に揃えた

### AddMemo

```swift
    let realm       = try! Realm()
    let imagePicker = UIImagePickerController()
```

・Realmは、理由はないけど、AddMemo内で使えるようにした

```swift
    @IBAction func addMemo(_ sender: Any) {
        //必要な変数を宣言
        let memoObject             = MemoModel()
        let archivedAttributedText = try! NSKeyedArchiver.archivedData(withRootObject: memoTextView.attributedText!, requiringSecureCoding: false)
        
        //入力値を代入
        memoObject.data       = archivedAttributedText
        memoObject.identifier = String().randomString()
        
        //データを追加
        try! realm.write{
            realm.add(memoObject)
        }
        
        //ViewControllerに戻る
        self.navigationController?.popViewController(animated: true)
    }
```
・宣言、代入、処理、に区別できるように空行を追加
・コメントで段落をつけ、段落の内容と範囲がわかるようにした
・クロージャでまとめるより、このままの方がわかりやすいので、クロージャは辞めた

```swift
    func imagePickerController(_ picker: UIImagePickerController, didFinishPickingMediaWithInfo info: [UIImagePickerController.InfoKey : Any]) {
        if let pickerImage = info[.originalImage] as? UIImage {
            //画像の圧縮やサイズ調整に必要な値
            let width                 = pickerImage.size.width
            let padding               = self.view.frame.width / 2
            let scaleRate             = width / (memoTextView.frame.size.width - padding)
            //圧縮した画像
            let resizedImage          = pickerImage.resizeImage(withPercentage: 0.1)!
            //インスタンスの宣言
            let imageAttachment       = NSTextAttachment()
            var imageAttributedString = NSAttributedString()
            //memoTextViewのテキストをAttributedStringに変換
            let mutAttrMemoText       = NSMutableAttributedString(attributedString: memoTextView.attributedText)
            
            //画像をNSAttributedStringに変換
            imageAttachment.image = UIImage(cgImage: resizedImage.cgImage!, scale: scaleRate, orientation: resizedImage.imageOrientation)
            imageAttributedString = NSAttributedString(attachment: imageAttachment)
            mutAttrMemoText.append(imageAttributedString)
            
            //画像を追加したAttributedStringをmemoTextViewに追加
            memoTextView.attributedText = mutAttrMemoText
        }
        //imagePickerを閉じる
        dismiss(animated: true, completion: nil)
    }
```

・宣言や代入が多いので、=を縦に揃えた
・他変数と揃えるため、paddingの型指定を排除
・値の代入、インスタンス&イニシャライザ、他処理、に分類
・各段落の内容を理解できるようにコメントを追加
・全ての変数は宣言時、pickerImage以外に利用する値がないので、順序を変えても大丈夫

### DisplayMemo

```swift
    //ViewControllerから値を受け取る変数群
    var selectedMemoObject          = MemoModel()
    var selectedMemo_attributedText = NSAttributedString()
    var selectedIndexPathRow        = Int()
```

・=を縦に揃えた
・初期化による型指定に変更
・コメントで、どのような役割を担うのかを説明

### EditMemo

```swift
    //Realmのインスタンス、Realmのデータを受け取る変数
    var memoList:Results<MemoModel>!
    let realm                       = try! Realm()
    //imagePickerのインスタンス
    let imagePicker                 = UIImagePickerController()
    //DisplayMemoから値を受け取る変数群
    var selectedMemoObject          = MemoModel()
    var selectedMemo_attributedText = NSAttributedString()
    var selectedIndexPathRow        = Int()
```
・=を縦に揃えた
・Realmの処理する度にインスタンスを生成するのは面倒なので、スコープをEditMemo全体に広げた
・ジェネリックはRealm指定のを使用しており、classとして定義していないので、初期化の書き方に変更できなかった

```swift
    override func viewDidLoad() {
        super.viewDidLoad()
        //imagePickerの設定
        imagePicker.delegate   = self
        imagePicker.sourceType = .photoLibrary
        
        //Realmからデータを取得、
        memoList = realm.objects(MemoModel.self)
        
        //memoTextViewにDisplayMemoから渡されたデータを代入
        memoTextView.attributedText = selectedMemo_attributedText
    }
```
・処理別に分類
・どのコードも=を縦に揃えることもできたが、処理の内容も担当する役割も全く違うので、区別のために揃えなかった

## まとめ

リーダブルコードを読むまで、コードの美しさとは
・少ないコード
・コメントが少ない
・短くてわかりやすい名前
・無駄な改行やスペースがない
を指すのだと思ってました。

ですが、
・**=を基準に揃える**
・**不可欠でもないコメントや空行**
・**同じ形式のコードに一貫性を持たせる**
を試してみて、地味で細かい作業も開発の1つだと気付きました。

オンラインサロンで自分のコードを見てもらう機会がありますし、これからもプログラミングに携わる身として、早いうちからコードを美しくする方法を知れてよかったです。

次回は、**コードのロジック**、を改善していきます。