リファクタリングに関するささやかなメモです。
今の時代には常識的なことにすぎないことです。
####推奨する指針
・変数の宣言は、その変数を使う直前まで遅くする。変数を宣言するときに同時に初期化する。
・ループの中でしか使わない変数の宣言は、ループの中で行う。
####推奨する形式のコード例
if(条件1){
cv::Mat mat;
somefunc1(mat);
}
if(条件2){
cv::Mat mat;
somefunc2(mat);
}
と書いてあれば、最初のif文のcv::Mat型の変数 matと2度目のif文のmat型は別物であることが明確にわかります。しかも2つのif文の後にmatの値を利用していないことがわかります(理由:文法的に利用できない)。
####推奨しない形式のコード例
しかし
IplImage *img;
if(条件1){
somefunc1(img);
}
if(条件2){
somefunc2(img);
}
//長い記述
//imgを使う記述があるかどうかは不明
と書いてあれば、条件1の処理後のimgが条件2で使われて、それが
2つのif文の後にも意味を保ち続けている。どこかで使われているかもしれないことを考えなくてはなりません。スコープが広がったこととIplImage型がポインタ渡しになっていることによって、コードの意味合いの可能性は格段に広がってしまっています。
変数のスコープを地道に狭めることも、ソースコードの理解を助けます。関数の抽出をする際には、関数の引数となる変数と関数内部での変数とに区別します。そのためにも、スコープが狭い方がよい。
###追記:for文を含むまとまりの関数化のヒント
変数のスコープを短くしていって、あるfor文の前と後で参照される値、for文の中でだけ参照される値が明確になったら、そのfor文を含むブロックの役割が明確になります。まずは、関数にしたらどういう関数になるか、コメント行で書いて見ましょう。
// double someFunction(const cv::Mat &img, cv::Mat &outImg);
みたいな感じで、書いてみて、@brief, @param[in], @param[out]などを含めて書いてみます。聞くだけで理解できる程度の意味合いの機能としてまとめて、その部分を関数化していきます。
###追記:"{"を使ってスクープを狭める
double r;
{
double sum=0.0;
int n=0;
for(int i=0; i<10; i++){
sum += x[i];
n += 1;
}
r = sum/static_cast<double>(n);
}
などとすれば、変数sum, n のスコープを{}の中に狭めることができます。
変数のスコープを狭めたときに、プログラムがコンパイルできて同じように実行できるならば、変数sum,nは、その後のコードで利用されていないことが確認できます。
関数化しないまでも{}を使って変数のスコープを狭めれば、ソースコードの理解がしやすくなって、メンテナンスも容易になります。
####こんなツールがほしい:
C++の各関数(メソッド)の中で、変数のスコープを可能なかぎり狭くするツール。
C++の関数の範囲の中でfor文、while文、if文、switch文などがあったときに、
・それぞれの文のブロックの前後で使用されていなければ、ブロックの中に押し込む。
・それぞれの文のブロックの中と後でしか使われていなければ、抽出すべき関数の戻り値として提案する。
・それぞれの文のブロックの中と前でしか使われていなければ、抽出すべき関数のinの引数として提案する。
・それぞれの文のブロックの中と前後で全て使われていれば、抽出すべき関数のin,outの引数として提案する。
だれか、そんなツール知りませんか?
参考URL
[レガシープログラマかどうかを判断する10項目]
(http://blog.jnito.com/entry/20110218/1297983647)