if-else文が多すぎます。リファクタリングする方法はありません

Question

これはリファクタリングできますか？またはこれは正常に見えます。 （変数名が変更された）

if (cmpScope.equals(GLOBAL)) { 
      return true; 
     } else if ((cmpScope.equals(X) || cmpScope.equals(Y)) 
       && cid == pid) { 
      return true; 
     } else if (cmpScope.equals(Z) && cid != pId) { 
      return true; 
     } else if (cmpScope.equals(V) && cid == pid) { 
      return true; 
     } else if (cmpScope.equals(Z) && cid == pid && cSubId != pSubId) { 
      return true; 
     } 
     return false; 

Answer 1

すべての式をor演算子で結合するのは、すべてtrueを返すためです。

return ((cmpScope.equals(GLOBAL) || 
     ((cmpScope.equals(X) || cmpScope.equals(Y)) && cid == pid) || 
     (cmpScope.equals(Z) && cid != pId) || 
     (cmpScope.equals(V) && cid == pid) || 
     (cmpScope.equals(Z) && cid == pid && cSubId != pSubId)); 

Answer 2

私は 'return true'は単なる例であると仮定していますか？それ以外の場合は、1つの式でtrueを返すすべてのステートメントをグループ化できます。彼らが実際に別のことをすることを意図しているなら、それは私にはうまく見えます。 （場合によっては、switch文を使用できますが、この場合は使用できませんが、一般的にはhttps://docs.oracle.com/javase/tutorial/java/nutsandbolts/switch.htmlを使用できることを知っておくとよいでしょう）

Answer 3

変数の条件の結果を格納し、すべての条件一つの声明で。各if句が値を返すので、あなたはすべてのelse Sを取り除くことができます

boolean trueCond1 = cmpScope.equals(GLOBAL); 
boolean trueCond2 = (cmpScope.equals(X) || cmpScope.equals(Y)) && cid == pid; 
boolean trueCond3 = cmpScope.equals(Z) && cid != pId; 
boolean trueCond4 = cmpScope.equals(V) && cid == pid; 
boolean trueCond5 = cmpScope.equals(Z) && cid == pid && cSubId != pSubId; 

return (trueCond1 || trueCond2 || trueCond3 || trueCond4 || trueCond5); 

Answer 4

（私はそれがすべての場合に、より良い解決策だとは思わないけど）。

if (cmpScope.equals(GLOBAL)) { 
    return true; 
} 
if ((cmpScope.equals(X) || cmpScope.equals(Y)) && cid == pid) { 
    return true; 
} 
if (cmpScope.equals(Z) && cid != pId) { 
    return true; 
} 
if (cmpScope.equals(V) && cid == pid) { 
    return true; 
} 
if (cmpScope.equals(Z) && cid == pid && cSubId != pSubId) { 
    return true; 
} 
return false; 

これはコードの見掛けの複雑さを軽減するための最良の手法の1つです。具体的なニーズに応じて、または演算子を使用して1つの式にグループ化する方が良い解決策かもしれません。また、CID/pidの比較が最後の4つのテストに共通しているので、このようなものが良いまだあるかもしれない：

if (cmpScope.equals(GLOBAL)) { 
    return true; 
} 
if (cid == pid) { 
    if (cmpScope.equals(X) || cmpScope.equals(Y)) { 
     return true; 
    } 
    if (cmpScope.equals(V)) { 
     return true; 
    } 
    if (cmpScope.equals(Z) && cSubId != pSubId) { 
     return true; 
    } 
} else { 
    if (cmpScope.equals(Z)) { 
     return true; 
    } 
} 
return false; 

Answer 5

私は完全にカール・manaster応答@サポートしていますが、私たちは先に進むことができると思います。

私のアプローチは、以下の原則に基づいています。

は

• 全くそうでない文があってはなりません。
• 可読性を高めるために、複雑な論理式をプライベートメソッドに抽出する必要があります。
• すべての可能な分岐をカバーするテストスイートがあります。このため、人々は循環器系の複雑さを心配しています。これがメソッドへの抽出が役立つ理由です。

例のコードから、このメソッドが正確に何をするかは不明です。そのため、いくつかのランダムな名前を選んだので、著者はそれをより有効なものに変更する必要があります。

私はテストや時間がなかったので、私はそれが動作する100％わからないんだけど、私は次のコードで終わってきた一連の変更後：

function(cid, pid, cSubId, pSubId) { 
    return cmpScope.equals(GLOBAL) && isValidSomething() && isValidSomethingElse(); 
} 

抽出方法は次のとおりです。

function isValidSomething() { 
    if(cid != pid) { 
     return false; 
    } 
    if (cmpScope.equals(V) || cmpScope.equals(X) || cmpScope.equals(Y)) { 
     return true; 
    } 
    return cmpScope.equals(Z) && cSubId != pSubId; 
} 

function isValidSomethingElse() { 
    return cmpScope.equals(Z) && cid != pId; 
} 

これは単なる例であり、さらに改善することができます。

また、この主なポイントは、「よく書かれた散文」のように読むことができるように、良い名前のプライベートメソッドを持つことです。これを想像してください：

function isMyCarValid() { 
    return isHaveWheels() && isHaveFuel() && isHaveDriver(); 
}