2016-07-21 4 views
2

Pmd가이 메소드 (thirdRowsValidation)의 복잡성은 14이지만 복잡하지는 않지만 코드를 줄일 수는 없다고 말했습니다.메소드의 복잡성을 줄입니다

는 indexBookngEnd, indexTravelStart ... 모든 변수 (CSV 파일의 열의 헤더) 다른 루프 반복에서 만들어진 다른 배열의 인덱스이다 - 지시 신호

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("| "); 


      } 

     } 

    } 

지시 신호를 :

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; 
      } 
\ 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); 
      } 

     } 

그래서 내가 무엇을 가지고 :: 617,451,515,- '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       | 
+0

루프의 내용을 별도의 메서드로 추출해야합니다. 그것은 복잡성을 줄여줍니다. 그런 다음 상당히 많은 필드 또는 전역 변수가있는 것으로 보아 해당 상태를 별도의 객체로 이동하면 약간 리팩토링 될 수 있으므로 해당 상태 객체를 이동할 수 있으므로 이후의 리팩터링이 조금 더 쉬워집니다. 그러나 이것들은 단지 추측 일뿐입니다. IntelliJ가 제안하는 것은 무엇이든간에 시작하십시오 :-) –

+0

그걸 이해합니다 ... 내가 여기 예를 들면서 만든 것 ...if (j == 0) {firstRowValidation (arrayPerRow); }하지만 알 수 있듯이 내가 바꿀 수없는 두 개의 메인 루프가 필요하다. – arnoldssss

+0

하지만 4 행에서 3 행까지 모든 것을 메서드로 옮길 수 있으므로'thirdRowsValidation'는'for'-loop 만 가질 것이다. 'i'의 모든 것에 대해 메소드를 호출하십시오. –

답변

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 
    } 
} 

메소드가 필요로하는 모든 것을 인수로 전달하십시오. 메서드는 다른 메서드에서 설정된 필드의 값에 의존해서는 안됩니다. 이렇게하면 복잡성이 줄어들고 코드를 읽고 이해하고 테스트 할 수 있습니다.

메소드 또는 클래스의 복잡성이 높은 경우 일반적으로 한 번에 너무 많은 작업을 시도합니다. 한 발 뒤로 물러나서 다른 작업을 식별하고 별도로 구현하십시오. 그러면 코드가 자동으로 복잡해집니다.

새로운 방법으로 내부 루프를 추출하는 것은 일반적으로 클래스의 복잡성을 줄이지 않고 단일 방법의 복잡성을 줄이는 데 도움이되는 빠른 해킹입니다.

+0

그것으로 작업 ... 아픈 내 결과를 말해 – arnoldssss

+0

감사합니다 아칸다 작품 – arnoldssss

1
당신은 별도의 방법으로 for -loop의 내부 논리를 이동하여 시작할 수

가 :

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 톰이 정의한 Clean Code 원칙입니다. 한 가지 일을하는 작은 메서드가 있습니다. 메서드는 이제 단일 행을 추출한 다음 다른 행을 호출하여 해당 행을 사용하여 무언가를 수행합니다. 즉 SRP (Single Responsibility Principle) 및 KISS (Keep It Simple, Stupid) 원칙을 준수해야합니다.