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

どうも、ねこきち（@nekokichi1_yos2）です。

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

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

今回は、コメントでコードを分かりやすく改善していきます。

リーダブルコードの教え

「コードからわかることはコメントしない」
・必要でもなく、無くても困らない
・同じ情報を2つも提示する必要はない
・新しい情報を提示してないなら、無価値

「自分の考えを述べる」
・詳しい情報を伝えられる
・読む人にどうして欲しいかが伝わる

「後で見返すためのコメント」
・コードの状態を視覚的に示す
・TODO、MUSTなどの単語

「何も書かないよりマシ」
・伝えたい情報が伝わればいい
・完璧なコメントを求めなくていい

「ハマりそうな罠を回避」
・コードの実行結果を伝える
・実行して確認する手間が省く
・罠を探す、罠の対処にかかる時間を短縮

「記号や数字でコメント」
・計算式、関係を表す式
・例：引数->インスタンス名

「簡潔で分かりやすいコメント」
・代名詞（あれ、それ、これ）は書くな
・接続詞など、余計な言葉は書くな
・曖昧で、冗長で、理解に手間取る

「実例を載せる」
・コードの実行結果
・扱う値やデータがどのように処理されるかがわかる

「一般的視点からコメント」
・一般人でも理解できる形
・例：〇〇が〜〜された
・実行結果を簡潔に伝えられる

「キーワードや用語で効率化」
・ITやエンジニアに共通用語
・例：〇〇処理、プロパティ
・長いコメントを代替えできる

変なコメントを探す

本記事を投稿する前に、あちこちのファイルに変なコメントをわざと追加しました。

ViewController

    //メモリスト
    var memoList:Results<MemoModel>!
    //Realm
    let realm               = try! Realm()
    //保存用の配列
    var attributedTextArray = [NSAttributedString]()

    override func viewWillAppear(_ animated: Bool) {
        super.viewWillAppear(animated)
        //初期化
        attributedTextArray = [NSAttributedString]()
        //データ取得
        memoList            = realm.objects(MemoModel.self)
        
        //Realmから取得したデータをAttributedStringに変換していって
        //attributedTextArrayに追加していく
        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：Realmの何なのか？

・保存用の配列：何を保存し、どこに保存するのか？

・初期化：何を初期化するのか？

・データ取得：どこから何を取得する？

・Realmから取得したデータをAttributedStringに変換していってattributedTextArrayに追加していく：冗長で何が言いたいのか簡潔に伝わらない

・tableView更新：どのtableViewを指すのか？

    func tableView(_ tableView: UITableView, commit editingStyle: UITableViewCell.EditingStyle, forRowAt indexPath: IndexPath) {
        //選択したメモのデータ
        let selectedMemo = memoList[indexPath.row]
        //TableViewで選択したメモのデータをRealmから削除する
        try! realm.write() {
            realm.delete(selectedMemo)
        }
        
        //tableView更新
        tableView.reloadData()
    }
    /***/
    
    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
        }
    }

・選択したメモのデータ：どこで選択した？

・TableViewで選択したメモのデータをRealmから削除する：どのTableViewなのか、少し長い

・DisplayMemoのインスタンスを宣言：内容が技術的寄りで、行われている過程が分かりづらい

AddMemo

//Realmに保存するためのデータの集まり
class MemoModel: Object {
    @objc dynamic var data: Data!
    @objc dynamic var identifier: String!
}

・Realmに保存するためのデータの集まり：具体的にどのようなデータかを説明した方がいい

    override func viewDidLoad() {
        super.viewDidLoad()
        
        //imagePickerの設定
        imagePicker.delegate   = self
        imagePicker.sourceType = .photoLibrary
    }

・imagePickerの設定：無くてもわかる

    @IBAction func addMemo(_ sender: Any) {
        //必要な変数を宣言
        let memoObject             = MemoModel()
        //memoTextViewに入力したテキストをData型に変換
        let archivedAttributedText = try! NSKeyedArchiver.archivedData(withRootObject: memoTextView.attributedText!, requiringSecureCoding: false)
        
        //入力値を代入
        memoObject.data       = archivedAttributedText
        memoObject.identifier = String().randomString()
        
        //Realmにデータを追加
        try! realm.write{
            realm.add(memoObject)
        }
        
        //戻る
        self.navigationController?.popViewController(animated: true)
    }

・必要な変数を宣言：コメントが示す変数が何なのか不明

・memoTextViewに入力したテキストをData型に変換：言い換えられないだろうか？

・入力値を代入：コメントが無くても理解できる

・Realmにデータを追加：addでデータを追加すると理解できる

・戻る：戻り先を示した方がいい

    //長押しタップで画像を添付
    @IBAction func attachImageGesture(_ sender: UILongPressGestureRecognizer) {
        //アラート
        let alert = UIAlertController(title: "画像を添付", message: nil, preferredStyle: .actionSheet)
        //アクション
        let action = UIAlertAction(title: "OK", style: .default) { (action) in
            self.dismiss(animated: true, completion: nil)
            self.present(self.imagePicker, animated: true, completion: nil)
        }
        //キャンセル
        let cancel = UIAlertAction(title: "キャンセル", style: .cancel, handler: nil)
        //アラートアクションを追加
        alert.addAction(action)
        alert.addAction(cancel)
        //表示
        present(alert, animated: true, completion: nil)
    }

・長押しタップで画像を添付：アラートを中継してるので、処理の流れを記述した方がいい

・アラート：アラートコントローラーの方が適切

・アクション：処理内容を示しても良い

・キャンセル：アクションとかを追加した方がいい

・アラートアクションを追加：コメントが無くても理解できる

・表示：何を表示する？

extension AddMemo: UIImagePickerControllerDelegate, UINavigationControllerDelegate {

    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)
    }

    func imagePickerControllerDidCancel(_ picker: UIImagePickerController) {
        dismiss(animated: true, completion: nil)
    }

}

・画像の圧縮やサイズ調整に必要な値：パラメーターとか、別の言い方が良さそう

・圧縮した画像：変数名でわかるし、圧縮率を示した方がいい

・インスタンスの宣言：何を宣言したのかを書くべき

・memoTextViewのテキストをAttributedStringに変換：テキストをメモに変えて、記号や英語で示した方が分かりやすそう

・画像をNSAttributedStringに変換：画像の詳細がないと、ImagePickerの画像なのか圧縮した画像なのかがわからない

・画像を追加したAttributedStringをmemoTextViewに追加：短く言い換えられそう

DisplayMemo

    //ViewControllerから値を受け取る変数群
    var selectedMemoObject          = MemoModel()
    var selectedMemo_attributedText = NSAttributedString()
    var selectedIndexPathRow        = Int()
    
    override func viewWillAppear(_ animated: Bool) {
        super.viewWillAppear(animated)
        
        //前画面で選択されたメモのテキストを表示
        memoTextView.attributedText = selectedMemo_attributedText
    }
    
    override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
        if segue.identifier == "edit" {
            //EditMemoのインスタンス
            let vc = segue.destination as! EditMemo
            //EditMemoのプロパティに代入
            vc.selectedMemoObject          = selectedMemoObject
            vc.selectedMemo_attributedText = memoTextView.attributedText
            vc.selectedIndexPathRow        = selectedIndexPathRow
        }
    }

・ViewControllerから値を受け取る変数群：ViewControllerで選択された、の方が分かりやすい

・imagePickerの設定：無くてもわかる

・前画面で選択されたメモのテキストを表示：変数の宣言領域でViewControllerから受け取ったとわかれば、コメントの必要がない

EditMemo

    @IBAction func updateMemo(_ sender: Any) {
        //編集したデータ
        let data2 = try! NSKeyedArchiver.archivedData(withRootObject: memoTextView.attributedText!, requiringSecureCoding: false)
        //Realm内にある編集した元データの指定先
        let memo  = realm.objects(MemoModel.self).filter("identifier == %@", selectedMemoObject.identifier!)
        
        //Realmに書き込み
        try! realm.write {
            memo.setValue(data2, forKey: "data")
        }
        
        //戻る
        self.navigationController?.popToRootViewController(animated: true)
    }

・編集したデータ：EditMemoの画面なので、編集したと書かなくても、データを編集していることは知っている

・Realm内にある編集した元データの指定先：Referenceの方がすっきりしてる

・Realmに書き込み：Updateの方が分かりやすい

extension UIImage {
    //データサイズを変更する
    func resizeImage(withPercentage percentage: CGFloat) -> UIImage? {
        //指定したパーセンテージの割合で画像を拡大/縮小する
        let canvas = CGSize(width: size.width * percentage, height: size.height * percentage)
        //リサイズ後の画像を返す
        return UIGraphicsImageRenderer(size: canvas, format: imageRendererFormat).image {
            _ in draw(in: CGRect(origin: .zero, size: canvas))
        }
    }
}

・データサイズを変更する：関数名でリサイズってわかるし、無くても良いかも

・指定したパーセンテージの割合で画像を拡大/縮小する：リサイズ用のCGSizeなので、サイズ情報とかの方が分かりやすい

・リサイズ後の画像を返す：返り値がUIImageなので、コメントが無くても、リサイズした画像を返すって理解できる

変なコメントを改善する

ViewController

    // Realmへ保存用のメモリスト
    var memoListForRealm:Results<MemoModel>!
    // Realm
    let realm               = try! Realm()
    // メモリスト
    var memoList            = [NSAttributedString]()
    
    override func viewWillAppear(_ animated: Bool) {
        super.viewWillAppear(animated)
        
        memoList                    = [NSAttributedString]()
        // Realm-Read
        memoListForRealm            = realm.objects(MemoModel.self)
        
        // memoList[i]: memoListForRealm.data -> NSAttributedString
        // memoList[i] add to attributedTextArray
        if memoListForRealm.count > 0 {
            for i in 0...memoListForRealm.count-1 {
                let attributeText = try! NSKeyedUnarchiver.unarchiveTopLevelObjectWithData(memoListForRealm![i].data) as! NSAttributedString
                memoList.append(attributeText)
            }
        }
        
        memoTableView.reloadData()
    }

・Realmへ保存用のメモリスト：複数のメモを持つだけでなく、Realmに保存するのが役割

・Realm：Realmのインスタンスは他に無く、Realmの処理の本体であることを明示

・メモリスト：本プロジェクト内で共通の呼び名として、メモリスト、とコメント

・Realm-Read：RealmからデータをRead(取得)することを簡単に示す

・memoList[i]: memoListForRealm.data -> NSAttributedString()：型の変換を矢印で、()で型（クラス）だと示す

・memoList[i] add to attributedTextArray：配列に追加するのを記号で表す方法を思いつけなかったので、英語でコメント

・memoTableView.reloadData()で、テーブルビューの更新だとわかるので、コメントは削除

    func tableView(_ tableView: UITableView, commit editingStyle: UITableViewCell.EditingStyle, forRowAt indexPath: IndexPath) {
        let selectedMemo = memoListForRealm[indexPath.row]
        
        //Realm-Delete
        try! realm.write() {
            realm.delete(selectedMemo)
        }
        
        tableView.reloadData()
    }
    /***/
    
    override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
        if segue.identifier == "display" {
            // DisplayMemoに選択したメモを渡す
            let vc = segue.destination as! DisplayMemo
            vc.selectedMemoObject          = memoListForRealm[memoTableView.indexPathForSelectedRow!.row]
            vc.selectedMemoString            = memoList[memoTableView.indexPathForSelectedRow!.row]
            vc.selectedIndexPathRow        = memoTableView.indexPathForSelectedRow!.row
        }
    }

・変数名で選択されたメモだとわかるので、コメントは削除

・Realm-Delete：Realmで削除を行うことを示す

・DisplayMemoに選択したメモを渡す：値渡しの処理をまとめて示す

AddMemo

// Model for Realm
class MemoModel: Object {
    @objc dynamic var data: Data!
    @objc dynamic var identifier: String!
}

・Model for Realm：Realm用のモデルであることを端的に明示

    override func viewDidLoad() {
        super.viewDidLoad()
        
        imagePicker.delegate   = self
        imagePicker.sourceType = .photoLibrary
    }

・imagePickerは1つの変数でのみ使用し、設定する処理も多くないので、コメントは抜いた

    @IBAction func addMemo(_ sender: Any) {
        let memoObject             = MemoModel()
        // memoTextView.attributedText -> Data()
        let attributedMemoData     = try! NSKeyedArchiver.archivedData(withRootObject: memoTextView.attributedText!, requiringSecureCoding: false)
        memoObject.data            = attributedMemoData
        memoObject.identifier      = String().randomString()
        
        // Realm-Add
        try! realm.write{
            realm.add(memoObject)
        }
        
        // Back to ViewController
        self.navigationController?.popViewController(animated: true)
    }

・memoTextView.attributedText -> Data()：プロパティ名も記載し、Data型への変換を示す

・Realm-Add：Realmへのデータ追加処理を示す

・Back to ViewController：頭文字を大にすることでクラスやオブジェクトだと示し、Back to、で〜へ戻ることを示す

    // 長押しタップ -> アラート表示 -> ImagePicker起動
    @IBAction func attachImageGesture(_ sender: UILongPressGestureRecognizer) {
        let alert = UIAlertController(title: "画像を添付", message: nil, preferredStyle: .actionSheet)
        let okAction = UIAlertAction(title: "OK", style: .default) { (action) in
            self.present(self.imagePicker, animated: true, completion: nil)
        }
        let cancelAction = UIAlertAction(title: "キャンセル", style: .cancel, handler: nil)
        alert.addAction(okAction)
        alert.addAction(cancelAction)
        present(alert, animated: true, completion: nil)
    }

・長押しタップ -> アラート表示 -> ImagePicker起動：矢印で複数の異なる処理を順序付きで示す。（異なる処理に個別にコメントをつけてもよかったが、簡潔に少ないコメントの方がすっきりして見やすい）

    func imagePickerController(_ picker: UIImagePickerController, didFinishPickingMediaWithInfo info: [UIImagePickerController.InfoKey : Any]) {
        if let pickerImage = info[.originalImage] as? UIImage {
            // NSAttributedString用のパラメーター
            let width                 = pickerImage.size.width
            let padding               = self.view.frame.width / 2
            let scaleRate             = width / (memoTextView.frame.size.width - padding)
            // 10%に圧縮した画像
            let resizedImage          = pickerImage.resizeImage(withPercentage: 0.1)!
            let imageAttachment       = NSTextAttachment()
            var imageAttributedString = NSAttributedString()
            // memoTextView.attributedText -> NSMutableAttributedString
            let mutAttrMemoString     = NSMutableAttributedString(attributedString: memoTextView.attributedText)
            
            // resizedImage -> NSAttributedString
            imageAttachment.image = UIImage(cgImage: resizedImage.cgImage!, scale: scaleRate, orientation: resizedImage.imageOrientation)
            imageAttributedString = NSAttributedString(attachment: imageAttachment)
            mutAttrMemoString.append(imageAttributedString)
            
            // 画像を追加後のテキスト -> memoTextView.attributedText
            memoTextView.attributedText = mutAttrMemoString
        }
        dismiss(animated: true, completion: nil)
    }

・NSAttributedString用のパラメーター：用途を記載し、パラメーターで単位や数値の役割であることを示す

・10%に圧縮した画像：実際の圧縮例で処理の中身を知れる

・resizedImage -> NSAttributedString()：変数resizedImageを変換する過程を示す

・画像を追加後のテキスト -> memoTextView.attributedText：最新のテキスト、と記載するより、どのようなテキスト、かを示した方が理解しやすい

DisplayMemo

    // ViewControllerから値を受け取る
    var selectedMemoObject          = MemoModel()
    var selectedMemoString = NSAttributedString()
    var selectedIndexPathRow        = Int()
    
    override func viewWillAppear(_ animated: Bool) {
        super.viewWillAppear(animated)
        
        memoTextView.attributedText = selectedMemoString
    }
    
    override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
        if segue.identifier == "edit" {
            // EditMemoに保持中のメモを渡す
            let vc = segue.destination as! EditMemo
            vc.selectedMemoObject          = selectedMemoObject
            vc.selectedMemoString          = selectedMemoString
            vc.selectedIndexPathRow        = selectedIndexPathRow
        }
    }

・ViewControllerから値を受け取る：受け取り元を変数名に含めてもよかったが、各変数に含めると名前が長くなるので、コメントの方に含めた

EditMemo

    @IBAction func updateMemo(_ sender: Any) {
        // memoTextView.attributedText -> Data()
        let editedMemoTextData = try! NSKeyedArchiver.archivedData(withRootObject: memoTextView.attributedText!, requiringSecureCoding: false)
        // Realm内のデータを参照
        let realmRef  = realm.objects(MemoModel.self).filter("identifier == %@", selectedMemoObject.identifier!)
        
        // Realm-Update
        try! realm.write {
            realmRef.setValue(editedMemoTextData, forKey: "data")
        }
        
        // to ViewContrller
        self.navigationController?.popToRootViewController(animated: true)
    }

・Realm内のデータを参照：何のデータかを示すこともできたが、コメントが長くなりそうなので、参照を表すことを記述

・Realm-Update：Realm内にあるデータを更新するので、Updateと記述

extension UIImage {
    func resizeImage(withPercentage percentage: CGFloat) -> UIImage? {
        // 圧縮後のサイズ情報
        let canvas = CGSize(width: size.width * percentage, height: size.height * percentage)
        // return resized Image
        return UIGraphicsImageRenderer(size: canvas, format: imageRendererFormat).image {
            _ in draw(in: CGRect(origin: .zero, size: canvas))
        }
    }
}

・圧縮後のサイズ情報：引数の圧縮率で算出されたCGSize(幅と高さ）だが、わかりやすく、画像のサイズに関する変数だと記述

・return resized Image：関数名と返り値の型で返り値の中身がわかるが、ぱっと見でわかるように、英語で記述

まとめ

今回、コメントを改善する中で、一部の変数名やインデントを修正しました。

コメントは文章や複数の記号を用いて複雑な説明を伝えることができますが、変数名よりも多くの情報を詰め込めるので、中途半端なコメントじゃ意味がありません。

変数名では伝えきれない情報を伝える、それがコメントの役割だと気付きました。

つまり、変数名とコメント、それぞれがお互いの情報伝達の欠点を補っているのです。

もちろん、インデントやコードのロジックも、情報を効率よく伝える要素の一つです。

ただ、変数名とコメントは、自由な形で情報を伝えられるので、時間をかけてでも取り組むべきだなと思いました。

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