元の記事はJavaなのでコメントに書くべきではないと思って記事として分けました。
必ずメソッドの最後でreturnすることの是非
個人的な意見ですが、僕も必ずメソッドの最後でreturnしようとするのはあまりよろしくないと思います。
言語は違いますが(これはJavaがどうこうって話じゃないと思うので)、例えば
public static bool IsLeapYearLast(int year)
{
bool result = true;
result &= year % 4 == 0; //4で割り切れる
result &= year % 100 != 0; //100で割り切れない
result |= year % 400 == 0; //400で割り切れたら100で割り切れてもよし
return result;
}
public static bool IsLeapYear(int year)
{
if (year % 400 == 0) return true; //400で割り切れたら確定
return year % 4 == 0 && year % 100 != 0; //400で割り切れないので、4で割り切れるかつ100で割り切れない
}
というメソッドがあったときに、これらに対しては
- 記事主さんが
どこでretが書き換えられているかわからない為、読み手はretの状態を記憶したまま、最後のreturnまで読み進めなければいけない。
と言っているように(今回はresultですが)、読み手には優しくないと思います。
IsLeapYearLast
でif文使ってないのはbool
で判定してbool
のresult
にfalse
だのtrue
だの入れるのは冗長だからです。
- メソッドを最後まで通さなきゃいけないような共通の処理があるなら呼び出し元でやれ
- 変数のスコープは短いほうがいじる期間が短いのでバグが混入しづらくてよい(これは後述の項にもつながる)
- 見方を変えれば
result
は関数内でグローバルとも取れるので、明らかにバグの混入率で言えば……
という僕の個人的な見解があります。
変数宣言は先頭でする
太古(規格の年代的に)のCの時代は、先頭で変数宣言しないといけなかったのでそれはしょうがないですが、今は現代です。
コメントで触れられている、「後から追加した変数名が被る」「同じ変数名でも役割が~」というのは記事にある
説明不足な変数名をつける
の項にもある通りその変数の役割を表せていないのが問題なので、そもそもの話被るような変数名が悪いとしか言えません。
現代のIDEには補完機能というものがあります。
メソッドの変数全てを記憶できる人間であれば全ての変数を覚えてからコードレビューができるので問題はありませんし、最初に全部書いてあった方が見やすいというのはわかります。
ただ、
public static Person GetPersonFirst(long countryId, long regionId, long personId)
{
Country country = null;
Region region = null;
Person person = null;
//数行の処理
country = GetCountry(countryId);
country.OnSelect();
region = country.GetRegion(regionId);
region.OnSelect();
person = region.getPerson(personId);
person.OnSelect();
return person;
}
よりかは
public static Person GetPerson(long countryId, long regionId, long personId)
{
//数行の処理
Country country = GetCountry(countryId);
country.OnSelect();
Region region = country.GetRegion(regionId);
region.OnSelect();
Person person = region.getPerson(personId);
person.OnSelect();
return person;
}
とかの方が見やすいですかね。左辺の型情報も必要な時にコードに出現するので。
(言語は関係ないのでvarで書けとかC#だと先頭でvar(型推論)使う時にnull代入するとエラーになるとかの見解を挟んではいけない)
※数箇所Countryがcontryになってたのを直しました
6:12追記
スコープは短いほどいいと言いましたが、例外もあります。
例えば、ConsoleクラスのWriteLineメソッドはstaticでグローバルなメソッドです。
これを無理にスコープというか、アクセスできる範囲を狭めようとすると、プロパティ経由で
Console.Out.WriteLine();
とか書くことになってしまいます。物事には例外というものがあります。