DateMessageProvider 코드에 대한 첫번째 해결 방법 및 이슈 제기

2013-01-09 11:59

"테스트를 모두 성공하려면 어떻게 해야 될까요?"(http://www.slipp.net/questions/50)에에) 대한 첫번째 접근 방법을 찾아보자.

package net.slipp;
 
import java.util.Calendar;
 
public class DateMessageProvider {
 
  public String getDateMessage() {
    Calendar now = Calendar.getInstance();
    int hour = now.get(Calendar.HOUR_OF_DAY);
     
    if (hour < 12) {
      return "오전";
    }
     
    return "오후";
  }
}

먼저 위 소스 코드의 문제점을 찾아보자. 위 소스 코드를 테스트하기 어려운 이유는 getDateMessage()에서 "Calendar now = Calendar.getInstance();" 코드를 통해 Calendar를 직접 생성하기 때문에 현재 시간을 변경할 수 없다는 것이 테스트를 힘들게 한다. 따라서 getDateMessage()에서 Calendar를 직접 생성하지 않고 메소드의 인자로 전달한다면 단위 테스트를 할 수 있지 않을까?

이 같은 생각으로 소스 코드를 변경해 보면 다음과 같다.

package net.slipp;


import java.util.Calendar;


public class DateMessageProvider {


	public String getDateMessage(Calendar now) {
		int hour = now.get(Calendar.HOUR_OF_DAY);


		if (hour < 12) {
			return "오전";
		}
		
		return "오후";
	}
}

위와 같이 Production Code를 변경한 후 Test Code를 다음과 같이 구현할 수 있다.

package net.slipp;


import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;


import java.util.Calendar;


import org.junit.Test;


public class DateMessageProviderTest {
	@Test
	public void 오전() throws Exception {
		DateMessageProvider provider = new DateMessageProvider();
		assertThat(provider.getDateMessage(createCurrentDate(11)), is("오전"));
	}


	@Test
	public void 오후() throws Exception {
		DateMessageProvider provider = new DateMessageProvider();
		assertThat(provider.getDateMessage(createCurrentDate(13)), is("오후"));
	}
	
	private Calendar createCurrentDate(int hourOfDay) {
		Calendar now = Calendar.getInstance();
		now.set(Calendar.HOUR_OF_DAY, hourOfDay);
		return now;
	}
}

위와 같이 구현하면 오전, 오후에 대한 단위 테스트를 모두 성공할 수 있다. 위와 같은 접근 방법은 DateMessageProvider 클래스의 getDateMessage() 메소드를 사용하는 곳이 많지 않거나, 프로젝트를 시작 단계부터 위와 같은 접근 방식으로 구현한다면 가능하다. 우리들이 Spring 프레임워크를 사용하면서 자주 사용하는 Dependency Injection 개념을 활용해 구현하면 된다.

그런데 새로 구현하는 시점이 아니고 이미 서비스를 오픈해서 운영하지 몇년이 지났으며 DateMessageProvider.getDateMessage() 메소드를 사용하는 부분이 너무 많아 대대적인 변경 작업을 해야한다면 어떻게 될까? 즉, Dependency Injection 개념으로 접근하기 힘든 상황이라면 어떤 방식으로 접근해야 할까? 레거시 코드에 Calendar와 같이 직접적으로 의존관계를 가지는 클래스가 많다면 어떻게 점진적으로 테스트를 할 수 있을까?

4개의 의견 from SLiPP

2013-01-09 18:24

null을 넘기는게 거시기 하지만.. 뭐 대충..

기존 메소드를 삭제할 필요는 없어보입니다.

```package net.slipp;

import java.util.Calendar;

public class DateMessageProvider { public String getDateMessage(){ return getDateMessage(null); } public String getDateMessage(Calendar now) { if( now == null ) now = Calendar.getInstance(); int hour = now.get(Calendar.HOUR_OF_DAY);

if (hour < 12) {
  return "오전";
}

return "오후";

}```

2013-01-09 19:06

이전 글부터 재밌게 읽었습니다. 퇴근 늦겠네요 ^^; 우선, 제가 생각한 방법들에 대해서 말씀 드려보면,

이전 댓글에서 나왔던, String getDateProvider(), String getDateProvider(Calenadar) 두 메소드를 오버로드 하고, 레거시코드에서 사용하지 않는 String getDateProvider(Calendar)의 scope를 default로 선언하는 방법이 가장 합리적인 것 같습니다. 인자의 Calendar 형을 int 로 변경해도 별 문제가 없을 것 같구요. 다만 레거시코드에서 사용하지 않는 getDateProvier(Calendar) 부분을 위한 테스트를 별도로 해 주어야 한다는 부분이 좀 신경쓰입니다.

그 이유는, getDateMessage(Calendar cal)부분을 따로 테스트해야 함인데, 이 부분이 public한 용도로 사용된다면 분명 분리하는 것이 합당하겠지만, getDateMessage()의 수행만을 테스트 하기 위한 목적이라면, 테스트의 본질이 약간 흐려질 수 있을 것 같습니다.

이를 위한 다른 방법으로는 필드를 하나 생성하는 방법이 있습니다. 클래스 또는 메소드의 덩치가 커서(사실 제 머리가 안좋아서;;) 메소드를 분리하기가 부담스러운 경우 사용합니다. 코드는 다음과 같습니다.

public class DateMessageProvider {


	DateMessageProvider(Calendar definedCalendar) {
		this.definedCalendar = definedCalendar;
	}


	Calendar definedCalendar;


	public String getDateMessage() {


		Calendar now = definedCalendar != null ? definedCalendar : Calendar.getInstance();


		int hour = now.get(Calendar.HOUR_OF_DAY);


		if (hour < 12) {
			return "오 전";
		}
		return "오 후";
	}
}

위의 필드가 신경쓰이는 경우는 다음과 같이 변경 할 수 있습니다.

public class DateMessageProvider {


	public String getDateMessage() {
		Calendar now = getCalendar();
		int hour = now.get(Calendar.HOUR_OF_DAY);


		if (hour < 12) {
			return "오 전";
		}
		return "오 후";
	}


	Calendar getCalendar() {
		return Calendar.getInstance();
	}
}

그리고 테스트에서는 getCalendar()에서 지정한 Calendar를 리턴하도록 오버라이드 합니다.

하지만 생성자나 메소드를 추가하기 보다는 가능한 필드를 두고 테스트에서 직접 대입하는 방법을 선호합니다.

두 경우 모두, 다음과 같이 레거시코드에서 생성되는 상황에 대한 테스트가 추가 되어야 한다고 생각하구요.

DateMessageProvider provider = new DateMessageProvier();
String dateMessage = provier.getDateMessage();
assertTrue( "오전".equals(dateMessage) || "오후".equals(dateMessage));

그런데, 테스트가 커지는 경우에는 위와 같은 방법들도 분명 문제가 있습니다. 각각 메소드에서 다양한 객체를 활용하는 경우, 테스트만을 위해서 Calendar, Date, long timeinMillis 같은 필드를 생성하게 되면 레거시 코드가 너무 복잡해집니다.

따라서, 이 경우는 인터페이스를 선언하고, deleagator를 활용해서 lagacy 코드의 가독성을 크게 해지지 않고 테스트를 수행 할 수 있을 것 같습니다.

public interface DateMessage{
    String getDateMessage();
    String getTimeMessage();
    boolean isFriday();
    ....
}
@Override
String getDateMessage(){
    if(delegator != null) return delegator.getDateMessage();
    //legacy 코드...
    return dateMsg;
}
@Override
String getTimeMessage(){
    if(delegator != null) return delegator.getTimeMessage();
    //legacy 코드...
    return timeMsg;
}
@Override
boolean isFriday(){
    if(delegator != null) return delegator.isFriday();
    //legacy 코드...
    return ifFriday;


...
}

legacy코드를 읽다가도 점프 할 부분이 delegator로 한정 되기 때문에, 코드 변경에 다른 부작용을 최소화 할 수 있다고 생각합니다. 추가적으로 메스드들의 의존관계가 얽혀있는 경우에도 테스트에서 오버라이드 해서 활용 할 수 있다는 점 때문에 종종 사용합니다.

그리고, 이전 글타래에서 언급하셨던, scope 문제는 저도 궁금한 점이 있습니다. 개인적으로 default는 privete으로 간주해도 괜찮을 것으로 판단하고, 실제 테스트에서도 적극적으로 활용하고 있었습니다. 제 경험이 미천하지만, package specific한 용도의 메소드를 접한 기억이 거의 없고, 현재까지 문제가 발생한 적도 없었고, 관련 아티클을 읽어봐도 무방하지 않나라는 의견을 접하는데, 혹시 제가 적극적으로 활용하지 못해서 쉽게 단정하는 것은 아닌지 가끔 의문이 들기도 합니다.

상품코드를 읽을 때에도 default는 private과 크게 다르지 않다고 간주하고 default는 private으로 가정하고 읽습니다. 만약 테스트에서 정말 호출 할 필요가 없는 경우에는, 지금까지의 습관 때문에 명시적으로 private으로 선언하는데, 최근 리플렉션역시 적극적으로 활용하는 추세에 비추어보면 정말 큰 의미가 있을까 고민 되기도 합니다.

2013-01-09 23:30

이전 문제에서 Dependency Injection을 잠시 생각했었는데 기존 인터페이스는 남겨둬야한다는 프레임에 갇혀 미쳐 생각 못했던 방법이네요. 역시 아는 만큼 보이는 것 같습니다^^

이번 이슈에 대한 제 생각은 우선 기존 레거시 코드에 대한 인터페이스를 유지해야하니 다음과 같은 코드가 되겠네요.

package net.slipp;


import java.util.Calendar;


public class DateMessageProvider {


	public String getDateMessage() {
		Calendar now = Calendar.getInstance();
		return getDateMessage(now);
	}
	
	public String getDateMessage(Calendar now) {
		int hour = now.get(Calendar.HOUR_OF_DAY);


		if (hour < 12) {
			return "오전";
		}


		return "오후";
	}
}

getDateMessage(Calendar now) 에 대한 테스트 코드는 이미 만들어져 있으니, 레거시 메소드인 getDateMessage() 에 대한 테스트 코드를 만들어 보면 될 것 같습니다.

어라? 그러고보니 문제가 다시 원점으로 돌아간건가요? ㅎㅎ Calendar 클래스에 대한 의존성 때문에 오전/오후 테스트를 만족 못시키는 상황이네요.

기존 복잡한 메소드에서 extract method 하여 좀 간단해졌으니 Dependency Breaking 하기 위해 fake 객체를 생성해보면 어떨까 생각해봤습니다. 테스트 코드는 다음과 같겠네요.

```package net.slipp;

import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*;

import java.util.Calendar;

import org.junit.Test;

public class DateMessageProviderTest { @Test public void 오전() throws Exception { DateMessageProvider provider = new DateMessageProvider(); assertThat(provider.getDateMessage(createCurrentDate(11)), is("오전")); }

@Test
public void 오후() throws Exception {
    DateMessageProvider provider = new DateMessageProvider();
    assertThat(provider.getDateMessage(createCurrentDate(13)), is("오후"));
}

@Test
public void 오전_레거시() throws Exception {
    DateMessageProvider provider = new DateMessageProvider() {
       public String getDateMessage() {
         return getDateMessage(createCurrentDate(11));
       }
    };


    assertThat(provider.getDateMessage(), is("오전"));
}

@Test
public void 오후_레거시() throws Exception {
    DateMessageProvider provider = new DateMessageProvider() {
       public String getDateMessage() {
         return getDateMessage(createCurrentDate(13));
       }
    };


    assertThat(provider.getDateMessage(), is("오후"));
}


private Calendar createCurrentDate(int hourOfDay) {
    Calendar now = Calendar.getInstance();
    now.set(Calendar.HOUR_OF_DAY, hourOfDay);
    return now;
}

}```

오전_레거시()와 오후_레거시() 코드를 보면 fake객체를 생성하여 테스트 메소드 부분을 overriding 했습니다. Calendar 와의 의존성을 제거하고 테스트하고자 하는 오전/오후 값을 반환받기 위해 해당 Calendar 객체를 생성해서 반환해주었습니다. 좀 더 복잡한 곳에서는 mock 객체를 사용하는 것도 방법일 것 같네요.

의견 추가하기

연관태그

← 목록으로