0

この質問はある程度好みの問題であることは承知しています。これは私が理解できないことではなく、他の人の意見を聞きたいだけであることは認めます。

ブール値と文字列の 2 つの引数を取るメソッドを作成する必要があります。ブール値はある意味では (すぐに明らかになるでしょう) 冗長ですが、メソッドが両方の引数を受け取る必要があり、ブール値が「間違った」値を持つ場合は特定のメッセージ テキストで例外を発生させる必要があるという仕様の一部です。 . bool は、文字列が null または空でないtrue 場合にのみ指定する必要があります。

そこで、同じことを (できれば!) 書くためのいくつかの異なるスタイルを示します。どれが最も読みやすく、優れたコーディング プラクティスに準拠していると思いますか?

// option A: Use two if, repeat throw statement and duplication of message string
public void SomeMethod(bool useName, string name)
{
  if (useName && string.IsNullOrEmpty(name))
    throw new SomeException("...");
  if (!useName && !string.IsNullOrEmpty(name))
    throw new SomeException("...");

  // rest of method
}


// option B: Long expression but using only && and ||
public void SomeMethod(bool useName, string name)
{
  if (useName && string.IsNullOrEmpty(name) || !useName && !string.IsNullOrEmpty(name))
    throw new SomeException("...");

  // rest of method
}


// option C: With == operator between booleans
public void SomeMethod(bool useName, string name)
{
  if (useName == string.IsNullOrEmpty(name))
    throw new SomeException("...");

  // rest of method
}


// option D1: With XOR operator
public void SomeMethod(bool useName, string name)
{
  if (!(useName ^ string.IsNullOrEmpty(name)))
    throw new SomeException("...");

  // rest of method
}


// option D2: With XOR operator
public void SomeMethod(bool useName, string name)
{
  if (useName ^ !string.IsNullOrEmpty(name))
    throw new SomeException("...");

  // rest of method
}

もちろん、他の可能性を提案することも大歓迎です。メッセージ テキスト"..."は、「'useName' が true の場合、名前を指定する必要があり、'useName' が false の場合、名前は許可されません」のようなものになります。

4

2 に答える 2

1

これらのトリックは、状況によっては読みやすさを向上させるために使用できますが、この場合、C、D、E は不適切なオプションであると言えます。これらは、2 つの異なる予想されるシナリオを取り、それらを 1 つの条件にまとめているため、わかりにくくなる可能性があります。あなたの意図/読者を混乱させます。これにより、コードの保守とデバッグが難しくなります (何が起こっているのかを理解するために少し考える必要があり、間違いを犯すリスクが高くなります)。

(私は D と E をさらに進めて、ブール値にバイナリ演算を適用しているため、それらも悪いと言います。つまり、bool (true/false) から安全に使用できる整数値への暗黙的な変換に依存していることを意味します)。私は暗黙の変換を避けることを好み、bool を bool として扱うように注意しています)

A、B は、予想される各シナリオの意味を順番に単純かつ明示的に説明しているため、より適切なオプションです。将来、コードを保守する人が、さまざまな条件下で予想されるコードの動作について混乱する可能性ははるかに低くなります。

ただし、コードを読みやすくするその他のオプションをいくつか示します。

bool nameIsValid = (!string.IsNullOrEmpty(name));
if (useName)
{
  if (!nameIsValid) 
    throw new SomeException("..."); 
}
else if (nameIsValid) 
{
  throw new SomeException("..."); 
}

(使用elseすると、リーダーが「useName」条件を再評価する必要がなくなり、読み取り時間が短縮されます)

または、よりコンパクトな方法で:

bool nameIsValid = (!string.IsNullOrEmpty(name));
if ((useName && !nameIsValid) || (!useName && nameIsValid))
    throw new SomeException("...");

(一時変数を使用すると、コードを圧縮/縮小して、コードをより読みやすくすることができますが、カバーしたい 2 つの別々のシナリオを明示的に指定することもできます)

あるいは：

bool nameIsValid = (!string.IsNullOrEmpty(name));

// If we are using a name it MUST be valid
if (useName && !nameIsValid)
    throw new SomeException("..."); 

// If we are NOT using a name, it is illegal to supply one
if (!useName && nameIsValid)
    throw new SomeException("...");

コメントを使用して、チェックの目的を明確にすることができます。これにより、コードを読んでいる人は、条件式が何を意味するかを考えなくても、コードが何を意味するかを理解できるようになります。(私が言いたいのは、それが何をするかを見るのは簡単だということですが、条件は、なぜこれをチェックしたいのかを!usename && nameIsValid説明していません.

この最後の例は私の個人的な好みです。実際にそれを実行するためのコードを書くだけでなく、意図したことと意図した理由を述べたからです。これにより、コードを読んでいるプログラマーは、私が言ったことを簡単に確認できます。簡単なクロスチェックができます。(このリスクは、誰かがコメントを更新せずにコードを変更できるため、コメントが誤解を招く可能性があると主張する人もいますが、ざっと目を通しただけで、コードとコメントが一致していないように見える場合は、あなたが知っている何か問題があり、そのコードを修正するために時間を費やす価値があるということです。それらが一致する場合、クイック クロスチェックは成功し、コードがおそらく意図したとおりであることを確信できます (したがって、実装ではなく、設計が正しいかどうかだけを考える必要があります)。

于 2012-10-08T22:10:08.053 に答える
1

メソッドが文字列が null か空かを知る必要がある場合は、それ自体をチェックできます。呼び出し元にチェックを強制する必要はありません。また、呼び出し元が (意図的または意図せずに) 間違ったブール値を渡したというリスクを冒す必要もありません。価値。そもそもそのようなブール値を渡す必要はありません。したがって、間違ったことをしないことがわかっているので、チェックする必要はありません。

そうは言っても、2 つのブール式を比較する必要がある場合、オプション C を選択しない理由はわかりません。A と B は単なる冗長です。コードを追加するためのコードを追加しています。そこにノーオペレーションを投げ込むようなものです。D は不必要に難解です。排他的な or がたまたま、必要なもの (等しいこと) の正反対 (つまり、ビットごとの NOT) であるという事実に依存する必要はありません( operator ==)。

于 2012-10-08T20:50:34.983 に答える