5
6

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

途中でreturnがどうこうとか宣言を先頭でするとか(C#)

Last updated at Posted at 2018-04-28

元の記事はJavaなのでコメントに書くべきではないと思って記事として分けました。

必ずメソッドの最後でreturnすることの是非

個人的な意見ですが、僕も必ずメソッドの最後でreturnしようとするのはあまりよろしくないと思います。
言語は違いますが(これはJavaがどうこうって話じゃないと思うので)、例えば

Leapyear.cs
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で判定してboolresultfalseだのtrueだの入れるのは冗長だからです。

  • メソッドを最後まで通さなきゃいけないような共通の処理があるなら呼び出し元でやれ
  • 変数のスコープは短いほうがいじる期間が短いのでバグが混入しづらくてよい(これは後述の項にもつながる)
  • 見方を変えればresult関数内でグローバルとも取れるので、明らかにバグの混入率で言えば……

という僕の個人的な見解があります。

変数宣言は先頭でする

太古(規格の年代的に)のCの時代は、先頭で変数宣言しないといけなかったのでそれはしょうがないですが、今は現代です。

コメントで触れられている、「後から追加した変数名が被る」「同じ変数名でも役割が~」というのは記事にある

説明不足な変数名をつける

の項にもある通りその変数の役割を表せていないのが問題なので、そもそもの話被るような変数名が悪いとしか言えません。
現代のIDEには補完機能というものがあります。
メソッドの変数全てを記憶できる人間であれば全ての変数を覚えてからコードレビューができるので問題はありませんし、最初に全部書いてあった方が見やすいというのはわかります。
ただ、

GetPerson.cs
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;
}

よりかは

GetPerson.cs
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でグローバルなメソッドです。
これを無理にスコープというか、アクセスできる範囲を狭めようとすると、プロパティ経由で

ConsoleOut.cs
Console.Out.WriteLine();

とか書くことになってしまいます。物事には例外というものがあります。

5
6
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
5
6

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?