読みづらいコードを見かけたから文句を言う
読みづらいコードを見かけたから文句を言う。
- 変数名が意味を表現していない。
- 例 :
itemNumber
・dataCount
。前者はアイテムの ID かなんかかと思いきや個数 (length
) を示すモノ、後者はループ中のindex
を示す変数名だった。 - 何が悪いの? : 何のために存在する変数なのか、どんな影響があるのか、いちいち調べて覚えておかないといけなくなる。誤解も生まれやすい。
- コメント : 名前重要。
- 例 :
- 意味に違いのない類語を混在させている。
- 例 :
inside
とinternal
の表現が混在するが同じ意味合いで使っている・使い分けに意図がない。 - 何が悪いの? : 読み手が表現の違いに理由があるのか調べて判断する手間が出てくる。変数名などで
grep
する時にモレやすい。 - コメント : 名前重要。
- 例 :
- 全くコメントを書かない。書いてあってもクラス名と同じ文字列が書いてある見出し風のコメント行のみ。
- 例 : かろうじて見つけたコメントが、設定用の XML ファイルに
<!-- HOGE Parameters -->
と書いてあるだけ。 - 何が悪いの? : コードから「Why」が分からない。なぜこのような実装にしたのか、何が狙いかが分からない。すなわち、単に読みづらいコードなのか、何らかの理由でそうせざるを得なかったのか判断が付かず、汚いコードが余計にリファクタリングしづらくなる。
- コメント : こういう場合は大抵
README
も書いていないので、コーディングした本人にロングインタビューしないと解読できない。
- 例 : かろうじて見つけたコメントが、設定用の XML ファイルに
- 概念的な共通点がないのに共通化する。
- 例 : 人間とクルマ、両方「走れる」から
Runner
クラスで共通化だー!みたいな。まぁ英語でいえばrun
とdrive
で違うんだけど、それにしたってRunner
クラスで共通化する意味が全くない。 - 何が悪いの? : 本質的に (抽象概念的に) 共通する要素ではないので、子クラスごとに親クラスの実装内容を読み替えないといけなくなる。読み手の脳内バッファが食われて理解に時間がかかる。
- コメント : 多分、目先のコードを共通化して使い回したいがためだけに、よく考えずにやらかしてる。「共通化」自体が正義だと思い込んでいる。
- 例 : 人間とクルマ、両方「走れる」から
- 早すぎる共通化のせいで共通コードの中に種類別の分岐がある。
- 例 : 先程の
Runner
クラスの中に、if 人間なら { … } else if クルマなら { … }
みたいなコードがある。 - 何が悪いの? : 汚いコードが不必要に散在して、リファクタリングが困難になっている。
- コメント : この時点で「あれ?共通化できてないな?」と思えないということは、抽象概念として捉えられていない。
- 例 : 先程の
- コードの表面的にも、論理的な構造にも、規則性がない。
- 例 : 空行の開け方に規則性がない。ディレクトリやファイルの分割単位に規則性がない。
- 何が悪いの? : 「コレの場合はこう」「アレの場合はこう」と、色々覚えておきながらコードを読む必要があり、読み手の負担になる。
- コメント : 「場当たり的」などというレベルではない不規則さ。わざと他の部分とは違う書き方をしようとしてるのだろうか。
書いた本人なら改修とかできるのかというと、本人も四苦八苦してる。もう捨てろよそんなコード、ってことで俺が全部書き直した。