処理を短くする

ねこもちです。

古いプログラムを修正していると、そのプログラムが非常に「長大」であると気づくことがあります。
一つの関数(メソッド)が数百・数千行に及んでいることもあります。

こういうのは、大体
「どこにあるデータがいつ更新されるのか、追うのが難しい」
「ifのネストが多く、ifとelse節の対比が難しい」
「途中でreturnすればいいのに、制御フラグに値をセットした上で最後まで処理を続けるので、処理を追うのが難しい」
「途中でreturnしているが、どの条件でそこに入るのかを追うのが難しい」
「同じようなコードが何回も出てくる(が、100%同じと断言できないので、結局すべて追う必要がある)」
など、その他のいろいろな「難しいポイント」を含んでしまっていることも多いのです。


■「どこにあるデータがいつ更新されるのか、追うのが難しい」
これについては、下記の方法を取ると、難しさが減ります。
「変数は使う直前で定義する」
「できるだけローカル変数にする」
「できるだけImmutableな設計にする」

たとえば、Webアプリケーション(ASP.NETなど)の例を挙げます。

public class XxxxPage : Page
{
private string _a = "";
private int _b = 0;

public void Page_Load(object sender, EventArgs e)
{
_a = Request.QueryString["a"];
if( string.IsNullOrEmpty(_a) ) {
return;
}
_a = _a.Trim();

if( IsPostBack )
{
_b = int.Parse(_a);
}
}

public void OnXxxxEvent(object sender, EventArgs e)
{
// _a, _bを使用するコード
}
}

このようになっていると、OnXxxxEvent()が呼ばれるときに、
・事前にPage_Load()が呼ばれていること
・Page_Load()内で、どのようなルールで_a, _bの値が決まるのかを知っていること
の2点がわかっていないと、OnXxxxEvent()を書くのが難しくなります。
また、他のメソッドが_a, _bの値を変更する可能性もあり、
結局すべてのプログラムを調べないと、挙動がわからないということになります。

public class XxxxPage : Page
{
public void Page_Load(object sender, EventArgs e)
{ // 何もしない。
}

public void OnXxxxEvent(object sender, EventArgs e)
{
string a = (Request.QueryString["a"] ?? "").Trim();
int b = !string.IsNullOrEmpty(a) && IsPostBack
? int.Parse(a)
: 0;
// a, b を使用するコード
}
}

このように、「使用する箇所で」変数を定義し、同時に初期化することで、aがなんなんか、bがなんなのか、
まぎれることがなく、プログラムが非常に明快です。

また、この書き換えの際に、元のコードでは
_a がまず Request.QueryString["a"]を指し、
つぎにそれをTrim()したものを指すようになっていましたが、
書き換え後ではaはRequest.QueryString["a"]をTrim()したものを指し、それを変更しないようにしています。
こうすることで、aという変数がいつでも同じ値になり、プログラムの挙動を理解するのが簡単になります。


その他についてはまた後日。