2016-11-07 4 views
0

우리는 1200 줄 길이의 메서드 (스레드 실행 메서드)가있는 레거시 응용 프로그램을 사용합니다. 이 방법은 대부분 긴 문장의 시퀀스가 ​​포함 된 단일 (사실) 동안입니다.코드 부분에 "Extract Method"리팩터링을 적용하는 방법

다음 C# 1 영역에있어서의 50 배 존재한다 :

#region Cancel pending 
    if (backgroundWorkerPrincipal.CancellationPending) 
    { 
     if (CanCancelThread) 
     { 
      ev.Cancel = true; 
      return; 
     } 
    } 
#endregion 

I 새로운 방법이 영역을 추출하는 올바른 가능한 경우 방식 궁금.

내가 말했듯이,이 단편 (영역)은이 방법 내에서 약 50 회 나타납니다. # region 내에서의 반품에 유의하십시오 (잠시 후 종료됩니다).

private void backgroundWorker_DoWork(object sender, DoWorkEventArgs ev) 

    while(true) { 

     ... 

     #region Cancel pending 
      if (backgroundWorkerPrincipal.CancellationPending) 
      { 
       if (CanCancelThread) 
       { 
        ev.Cancel = true; 
        return; 
       } 
      } 
     #endregion 

     ... 

     #region Cancel pending 
      if (backgroundWorkerPrincipal.CancellationPending) 
      { 
       if (CanCancelThread) 
       { 
        ev.Cancel = true; 
        return; 
       } 
      } 
     #endregion 

       ... 

     #region Cancel pending 
      if (backgroundWorkerPrincipal.CancellationPending) 
      { 
       if (CanCancelThread) 
       { 
        ev.Cancel = true; 
        return; 
       } 
      } 
     #endregion 

       ... 

     #region Cancel pending 
      if (backgroundWorkerPrincipal.CancellationPending) 
      { 
       if (CanCancelThread) 
       { 
        ev.Cancel = true; 
        return; 
       } 
      } 
     #endregion 

     ... 

     #region Cancel pending 
      if (backgroundWorkerPrincipal.CancellationPending) 
      { 
       if (CanCancelThread) 
       { 
        ev.Cancel = true; 
        return; 
       } 
      } 
     #endregion 

     . 
     . 
     . 

    } 

} 
+0

실제로 이것이 자바로 태그되어야합니까? – nbrooks

+0

@nbrooks 멋진 잡기. –

+0

.net 4.5에서이 'BackgroundWorker'는'Task','async' 및'await'로 대체 될 수 있습니다. 시도 해봐. –

답변

3

내가 그것을 추출을 포함하여, 작동을 할 수있는 좀 펑키 한 가지 방법을 리팩토링하는 올바른 방법이 있다고 언급하지 않았다 :

그래서 방법은 다음과 같은 구조를 가지고 제어문을 실행할지 여부를 true/false로 반환하는 메서드. 당신은 여전히 ​​그것을 50 번 반복해야하므로, 그렇게하는 것이별로 이득이 아닙니다.

해당 코드 블록을 리팩토링하는 것을 전혀 권장하지 않습니다. 대신, 저는 리팩터링 코드 주위에 코드을 제안 할 것입니다. 인접한 코드 조각을 리팩토링하면 이전에 식별 할 수 없었던 패턴이 나타날 수 있습니다.

각 "..."블록에 대한 메소드를 추출하여 시작하십시오. 지금 당신이 가지고있는 것은 "방법을 호출 한 다음 취소가 보류 중일 때 보석금을내는"패턴입니다. 이러한 메서드를 대리자로 변환하면 데이터 요소가되고 루프에서 반복 할 수 있습니다.

추출 된 메소드가 동일한 서명을 가지고 있다고 가정 해 봅시다. 델리게이트 인스턴스의 배열을 선언하고, 루프에서 실행하고, 각 반복의 끝에서 보류중인 취소를 확인합니다. 휴식 대신 복귀가 있으므로 내부 루프에서 벗어나기 위해 추가 작업을 수행 할 필요가 없습니다.

var extractedMethods = new Func<State, DoWorkEventArgs, State>[] 
{ 
    DoStep1, 
    DoStep2, 
    DoStep3, 
    // ... 
}; 

while (true) 
{ 
    foreach (Func<State, DoWorKEventArgs, State> fn in extractedMethods) 
    { 
     state = fn(state, ev); 

     if (backgroundWorkerPrincipal.CancellationPending && CanCancelThread) 
     { 
      ev.Cancel = true; 
      return; 
     } 
    } 
} 

"방법을 호출 한 후 취소 보류중인 경우 구제"지금은 전화를 무슨 방법의 목록에서 분리되고 만 유지 한 취소 수표를 가지고있다. 메소드 목록이 정립 된 다음 해당 블록에 제공됩니다. while 루프를 자체 메서드에 추출한 다음 대리자 목록을 전달하는 추가 단계를 수행 할 수 있습니다. 반면에, 그것은 당신의 요구에 너무 멀어 질 수도 있습니다.

추출 된 메서드의 서명이 다른 경우 간단하지는 않지만 몇 가지 옵션이 있습니다. 같은 매개 변수를 사용하고 사용하지 않는 매개 변수는 무시하도록 메서드를 조정할 수 있습니다. 하지만 매개 변수가 너무 많아서 유지 보수성이 떨어질 수 있습니다. 특히 50 가지 방법을 조정해야하는 경우 특히 그렇습니다. 앞으로 더 많은 매개 변수 변형의 필요성을 예상한다면 이는 좋은 선택이 아닐 것입니다.

또 다른 옵션은 상대적으로 단순한 서명으로 람다를 사용하고 차이를 추상화하기 위해 클로저를 이용하는 것입니다.

var extractedMethods = new Func<State, DoWorKEventArgs, State>[] 
{ 
    (st, ev) => RunStep1(st, ev /*, parameters specific to RunStep1 */), 
    (st, ev) => RunStep2(st, ev /*, parameters specific to RunStep2 */), 
    (st, ev) => RunStep3(st, ev /*, parameters specific to RunStep3 */), 
    // ... 
};