1

次のコードブロックがあります。

if (x > 5)
{
    if (!DateTime.TryParse(y, out z))
        break;
    if (w.CompareTo(z) == -1)
        break;
}

x は整数、y は文字列、z と w は DateTime 変数です。その理由は、break;ブロック全体がループ内にあるためです。

これを単純化して読みやすくする方法はありますか？

4

7 に答える 7

3

コードを実行するために複数のブロックは必要ありませんif。ループを実行するか、ループを実行しないか (if と else) の 2 つのうちの 1 つだけを実行するためです。ここに示すように、単一のブール式を使用して、そのループの反復をスキップするかどうかを表すことができます。

(x > 5) && (!DateTime.TryParse(y, out z) || w.CompareTo(z) == -1)

そうは言っても、このような複雑な条件をループ内に含めると、可読性が損なわれる可能性があります。個人的には、ループが次のようになるように、この条件をメソッドに抽出するだけです。

while(!done) // or whatever the while loop condition is
{
    if(itemIsValid(x, y, w, out z))
    {
        //the rest of your loop
    }
}

//it may make sense for x, y, w, and possibly z to be wrapped in an object, or that already may be the case.  Consider modifying as appropriate.
//if any of the variables are instance fields they could also be omitted as parameters
//also don't add z as an out parameter if it's not used outside of this function; I included it because I wasn't sure if it was needed elsewhere
private bool itemIsValid(int x, string y, DateTime w, out DateTime z)
{
    return (x > 5) 
        && (!DateTime.TryParse(y, out z) || w.CompareTo(z) == -1)
}

これにはいくつかの利点があります。まず、コメントを必要とせずにコードを自己文書化する方法です。ループを見ると、「私が終わっていない間に、アイテムが有効な場合は、このすべてを実行してください」と読むことができます。有効性がどのように定義されているかに興味がある場合は、メソッドを調べてください。そうでない場合はスキップしてください。メソッドの名前を「isReservationSlotFree」など、より具体的なものに変更することも、これが実際に表しているものに変更することもできます。

検証ロジックが複雑な場合 (これは多少複雑です)、より複雑なループを乱雑にすることなく、コメントや説明を追加できます。

于 2012-09-27T19:10:55.150 に答える
2

「簡略化」とは、読みやすいという意味ではありません。

次の方法で、コードを読みやすくする (そして、さまざまなコーディング ルールに関してより安全にする) ことができます。

1) if ステートメントなどには常に角かっこを使用する

2) 「!」の使用を避ける (「== false」の方がより明確です)

3) それらの変数が何であるかを明示する変数名を使用します。

4) 複数の break ステートメントを避ける。代わりに、while の条件で評価されるフラグを使用します。

5) コードがまだ読みにくい場合: コメントを使用してください。

于 2012-09-27T18:14:32.057 に答える
2 
if (x > 5)
{
    if(!DateTime.TryParse(y,out z) || w.CompareTo(z) == -1)
        break;
}

2 つの条件は同じ結果になるため、1 つに結合できます。

于 2012-09-27T17:42:11.340 に答える
2 
if ((x > 5) && (!DateTime.TryParse(y, out z) || w.CompareTo(z) == -1))
    break;
于 2012-09-27T17:42:51.713 に答える
1

さらに重要: 説明的な変数名を使用w, x, y, zしてください (うまくいけば、これらの名前は単なる例です):

の代わりに、小なりまたは大なり演算子を使用することもできますCompareTo。

if (x > 5)
{
    bool isValidDate = DateTime.TryParse(y, out z);
    if (!isValidDate || z > w)
    {
        // comment like: stop processing if the current date 
        // is after the reference date, or if there was a parsing error
        break;
    }
}
于 2012-09-27T18:03:07.733 に答える
1

これがもう1つのバージョンです。 

var Break = x > 5 ? ((!DateTime.TryParse(y, out z) || w.CompareTo(z) == -1) ? true : false) : false;

短いが可読性を妨げる。

于 2012-09-27T18:40:38.783 に答える
0 
if ( x > 5 ){
    if (!DateTime.TryParse(y, out z) || w.CompareTo(z) == -1) break;
}
于 2012-09-27T17:54:07.887 に答える