중복코드 리팩토링 어떻게 하십니까?

의견 추가하기↓ / SLiPP 최신의견↓ / 페북 최신의견↓
bluepoet 2013-04-04 11:08

얼마전 회사에서 코드를 짜다가 다음과 같은 이슈를 만났습니다.

public Test getResult() {
   Specification spec = TestSpecs.search();
   Test t = TestRepository.findOne(spec);


   return t;
}


public String getResult() {
   Specification spec = TestSpecs.search();
   Test t = TestRepository.findOne(spec);
   
   if(t == null) {
      return "검색결과가 없습니다.";
   }


   return t.getSearchResult();
}

위 두개의 메서드에서 중복되는 부분은

Specification spec = TestSpecs.search();
Test t = TestRepository.findOne(spec);

그래서 아래 두가지 방법으로 리팩토링을 하면 어떨까 생각했습니다.

(1) 중복코드를 기존 메서드로 대체하는 방법

public Test getResult() {
   Specification spec = TestSpecs.search();
   Test t = TestRepository.findOne(spec);


   return t;
}


public String getResult() {
   Test t = getResult();
   
   if(t == null) {
      return "검색결과가 없습니다.";
   }


   return t.getSearchResult();
}

(2) 바뀌는 부분에 대한 추상화를 통해 인터페이스를 두고 전략패턴 활용해 다르게 행동하는 것

public interface ResultStrategy {
    Object getResult(Test t);
}


public Test getResult() {
   ResultStrategy resultStrategy = new ResultStrategy() {
      @Override 
      public Test getResult(Test t) { 
          return t;
      }
   };


   return (Test)getTemplateMethod(t, resultStrategy);
}


public String getResult() {
      ResultStrategy resultStrategy = new ResultStrategy() {
          @Override 
          public Test getResult(Test t) { 
              if(t == null) {
                  return "검색결과가 없습니다.";
              }


              return t.getSearchResult();
          }
       };


       return (String)getTemplateMethod(t, resultStrategy);
}


private Ojbect getTemplateMethod(Test t, ResultStrategy  strategy) {
   Specification spec = TestSpecs.search();
   Test t = TestRepository.findOne(spec);
    
   return strategy.getResult(t);
}

여기서 작성한 예제는 너무 간단한 것이라 사실 (1)번으로 하는게 가독성면에서도 좋고,

오버엔지니어링 하지 않는 것이라 생각되지만, 이외에도 다양하게 리팩토링 하는 방법이 있을 것 같아

글을 올립니다. 다른 분들은 보통 이렇게 중복코드가 나올시 어떤 방식으로 리팩토링을 하시나요???

0개의 의견 from FB

4개의 의견 from SLiPP

benghun 2013-04-04 14:52

제 생각은 이렇습니다. 메서드가 null을 반환하는 것 자체가 NPE를 발생시킬 위험이 크기 때문에 피하는게 좋을 것 같고요. 클라이언트가 t.getSearchResult()를 알아도 무방할 것 같아요. 지금 상황만 봐서는 이미 Test 클래스를 알고 있을 듯 하니까요.

한 클래스에 위 2개의 메서드가 존재한다면 아마도 문법적 오류일테지만, 2개의 클래스라고 가정하더라도 개념적인 측면에서 봤을 때 개선할 여지가 있을 것 같네요. 한 도메인에서 동일한 이름으로 다른 반환값을 가지는 것은 일관성에 좋지 않아보이구요. public Test getResult() public String getResult()

DDD에서는 bounded context 내부에서는 설계원칙들을 철저하게 지킬 필요는 없다고 하죠. 어차피 강하게 결합되어야 할 그룹이니까요. 너무 많은 것을 숨기고자 하는 것도 좋지는 않은 것 같아요.

benghun 2013-04-04 14:56

저 같으면 이거 하나로 퉁칠 것 같아요.

Test getResult() {
    Specification spec = TestSpecs.search();
    Test t = TestRepository.findOne(spec);


    if (null == t) t = Test.getEmpty();
    return t;
}


void caller() {
    Test t = getResult();
    sysout(t.getSearchResult());
}
bluepoet 2013-04-08 17:55

@benghun (1)번 방법과 거의 동일한 방법이네요. 의견 고맙습니다.

@양완수 제 생각엔 위 예제의 경우 클래스를 따로 뺄 정도의 규모는 아닌 것 같습니다. 의견 고맙습니다.

의견 추가하기

연관태그

← 목록으로