メソッドの複雑さを減らす

Question

Pmdは私にこのメソッド（thirdRowsValidation）の複雑さは14だと言いましたが、コードを減らすための方法はありません。

public void secondRowValidation(String secondRowCsv) { 

     String[] lines1 = secondRowCsv.split(","); 
     for (int i = 0; i < lines1.length; i++) { 

      if ("Bookng start".equalsIgnoreCase(lines1[i])) { 
       indexBookngStart = i; 
      } 
      if ("Bookng end".equalsIgnoreCase(lines1[i])) { 
       indexBookngEnd = i; 
      } 

：Ref1の

public void thirdRowsValidation(String thirdRowCsv) { 
     String[] lines1 = thirdRowCsv.split(","); 
     for (int i = 0; i < lines1.length; i++) { 

      if (indexBookngEnd == i && "".equals(temporalValidateBookngEnd)) { 
       temporalValidateBookngEnd = (" to " + lines1[i] + "\n"); 

      } 
      if (indexBookngStart == i 
        && !("".equals(temporalValidateBookngEnd))) { 
       finalOutput.append("Then it should have booking window "); 

       indexBookngStart = -1; 

      } 

      if (indexTravelEnd == i && "".equals(temporalValidateTravelEnd)) { 
       temporalValidateTravelEnd = (" to " + lines1[i] + "\n"); 

      } 

      if (indexTravelStart == i 
        && !("".equals(temporalValidateTravelEnd))) { 

       finalOutput.append("Then it should have travel window "); 

       String idHeaderColumn = String.format("%1$-" + 5 + "s", "id"); 
       String typeHEaderColumn = String.format("%1$-" + 50 + "s","type"); 
       finalOutput.append("| "); 
       finalOutput.append(idHeaderColumn); 

       indexTravelStart = -1; 
      } 

      if (indexPackageDescription == i) { 
       temporalPackages = String.format("%1$-" + 50 + "s", lines1[i]); 

      } 

      if (indexPackageCode == i 
        && !(lines1[i].matches("[+-]?\\d*(\\.\\d+)?")) 
        && indexTravelStart == -1) { 

       finalOutput.append("| "); 


      } 

     } 

    } 

Ref1の -

indexBookngEnd、indexTravelStart ...すべてのこれらの変数は、他のループ反復（CSVファイルの列のヘッダー）で行われた他のアレイからのインデックスであります\ nから、後でのため

アレイ ""

public String getStoryFromCsv(String convert) { 
     String[] lines = convert.split("(\n)"); 
     for (int j = 0; j < lines.length; j++) { 

      arrayPerRow = lines[j]; 
      if (j == 0) { // get marketing type and number 
       firstRowValidation(arrayPerRow); 
      } 
      if (j == 1) { // get headers 
       secondRowValidation(arrayPerRow); 
      } 
      if (j > 1) { // get info and append according to headers 
       thirdRowsValidation(arrayPerRow); 
      } 

     } 

は、だから私が持っているものです：：- 「thirdRowsValidation」メソッドアイデアを持っている私はあなたたちのために、このようなテキストに達してい終わりに14

の循環的複雑度を持っている - メソッドthirdRowsValidationは（）649 のNPATH複雑性を有します

Then it should have booking window 8/8/16 to 10/8/16 
Then it should have travel window 11/6/16 to 12/25/16 
And it should have online packages: 
| id | type            | 
| 34534 | aaa Pkg           | 
| D434E | MKW Pkg + asdasdasdasdasdasdas     | 
| F382K | sds Pkg + Ddding         | 
| X582F | OYL Pkg + Deluxe Dining       | 

Answer 1

この方法の複雑さは、それがさまざまなことをたくさんしているので高いです。メソッドごとに1つを試してみてください。

public String getStoryFromCsv(String csv) { 
    StringBuilder story = new StringBuilder(); 
    String[] lines = csv.split("\n”); 
    appendBookingWindowValidation(story, lines[0]); 
    appendTravelWindowValidation(story, lines[1]); 
    appendOnlinePackageValidation(story, lines); 
    return story.toString(); 
} 

private void appendBookingWindowValidation(StringBuilder story, String firstLine) { 
    story.append("Then it should have booking window "); 
    // extract start and end date from 'firstLine' 
    // and append them to the story 
} 

private void appendTravelWindowValidation(StringBuilder story, String secondLine) { 
    story.append("Then it should have travel window "); 
    // extract start and end date from 'secondLine' 
    // and append them to the story 
} 

private void appendOnlinePackageValidation(StringBuilder story, String[] lines) { 
    story.append("And it should have online packages:\n") 
     .append("| id | type            |\n"); 
    for (int i = 2 ; i < lines.length; i++) { 
     // create and append a row of the online packages table 
    } 
} 

メソッドが必要とするすべてのものを引数の1つとして渡すようにしてください。メソッドは、別のメソッドで設定されているフィールドの値に依存するべきではありません。これにより、複雑さが軽減され、コードの読み込み、理解、テストが容易になります。

メソッドやクラスの複雑さが高い場合は、通常、一度に多すぎることを試みることを意味します。一歩踏み出して、それが何であるかを特定して別々に実装しようとする。これにより、複雑さの低いコードが自動的に生成されます。

新しいループに内部ループを抽出するのは、通常、クラス全体の複雑さではなく、単一のメソッドの複雑さを軽減するのに役立つ高速ハックです。

Answer 2

あなたが別の方法にfor - ループの内部ロジックを移動することから始めることができます：

public void thirdRowsValidation(String thirdRowCsv) { 
    String[] lines1 = thirdRowCsv.split(","); 
    for (int i = 0; i < lines1.length; i++) { 
     doSomethingWithTheRow(i, lines[i]); 
    } 
} 

そしてdoSomethingWithTheRow()メソッド内で、あなたの内側のコードが存在します：

doSomethingWithTheRow(int i, String row) { 
    if (indexBookngEnd == i && "".equals(temporalValidateBookngEnd)) { 
     temporalValidateBookngEnd = (" to " + row + "\n"); 

    } 
    if (indexBookngStart == i 
       && !("".equals(temporalValidateBookngEnd))) { 
    ... 

複雑さを許容範囲内に抑えることができるかどうかはわかりませんが、最初の開始です。また、それはBobのおじさんによって定義されたクリーンコード原則です。あなたは小さなメソッドを持っています。一つのことをしています（メソッドは単一行を抽出して、その行で何かを行う他のメソッドを呼び出します）。つまり、SRP（単一責任原則）とKISS（Keep It Simple、Stupid）の原則です。