最近、ソースの品質についてばかり考える毎日。。。
典型的なオブジェクト指向を想定して”メソッド”ってことにしますが、
functionなりプロシージャなりに読み替えても同じです。
「1メソッドに何行まで書いていいか」はよく話題になりますよね。
場所によってはコーディング規約で〇行までって決まっていたり。
まあ、標準としては定量的に〇行って決まってたほうが分かりやすかったりしますが、
行数って本質じゃあないと思うんです。。。
要は、
メソッドが高凝縮になっている、
言い換えると、
「このメソッドが何をしているか」を"一言で"説明できるのであれば、
別に100行書こうが1000行書こうがいいと思うわけです。
じゃあ逆に、
凝縮度が低くて読むのも苦痛なスパゲッティコード（怒）ってどういうものなのか、
考えてみましょう。
- 変数がやたら多い
メソッドの引数
ローカル変数
グローバル変数（そもそも最低限にしてくれ...）
のトータルを意味しています。
※たまにクラスのフィールドをまるでグローバル変数のように使っている人がいますが、
お願いなのでオブジェクト指向をちゃんと勉強してください。。。
まあ、例えば
public void execute(String arg1, String arg2, ...){//10個くらいの引数
bool flag;
String temp1;
String temp2;
ArrayList<String> arrA;
ArrayList<String> arrB;
//...ずらずらと10個以上変数宣言
//そこから100行以上処理
}
...見た瞬間読む気失せるわ!!
例外もあるにしても、
引数が10個超えたり頭で10個以上ローカル変数を宣言したらその時点で設計を疑いましょう!
ましてや上記のように、
ましてや上記のように、
arg1,arg2とか、arrA,arrBとか
変数に枝番なんか付けだした日には最悪です!
枝番付けるくらいなら速やかにprivateなサブメソッドを書いてくれ!!
2. 変数の再定義が多すぎる
いや、多すぎるっていうか、個人的には1回でも再定義が必要になったら
疑うところだと思いますが。。。
public void execute(){
String t;
if(something == true || condition == true){
t = "0";
}elseif(hoge == false){
t = "1";
}else{
t = "2";
}
//...100行くらい処理
t = "0"; //えっ...!?
//全然違う条件で再定義.。。。
if(hoge == true){
t = "1";
}else{
t = "3"; //しかも3ってなんだよ。。。どっから出てきたんだよ。。。
}
//また100行くらい処理。。。
}
もう、敢えてセキュリティ上の理由かなんかで難読化してるとしか思えない。。。
頼むからスコープの中でくらい変数の意味を統一してくれ！
で、ハラが立って「再定義禁止！」とか言うと、今度は、
t1,t2,t3とかが増殖し出す始末。。。
きっと「メソッドを分離すると風邪を引く」みたいな迷信をおばあちゃんに教えられたに
違いない。。。
3. ループやtry-catchの中が長い
冒頭で何行でも書いていいとは言いましたがこれは流石に。。。
public void execute(){
String[] arr1 = getArray1();
String[] arr2 = getArray2();
//まずforeachで書けないか疑ってくれ。。。
for(int i = 0;i < arr1.length; i++;){
try{
//for - try - for のネスト。。。
for(int j = 0; j < arr2.length; j++;){
//100行くらい処理
//こういうときに限ってさらにifのネストも深かったりする
}//ここを読む頃にはもうどこからループが始まったのやら。。。
}catch(Exception e){
//しかもException握り潰すんだ。。。
}
}
}
ここまでくると、書いた人の頭の中覗いて見たいレベル。。。
このメソッドの処理を正しく理解するのは諦めるより他無いですな。。。
はじめの{から終わりの}までが画面上ワンスクロール以内に終わらなかったら分割しましょう！
...だからと言って巨大な縦長ディスプレイ買ってもダメです。
結論として、上記のような条件に当てはまらなければ、
別に100行書こうが1000行書こうがいいと思いますよ。。。
そんな努力するくらいなら分割した方が早いけど。。。
