요즘 자바를 이용해서 라기 보다 이래 저래 프로그램을 만들어 보고 글도 읽고 고민도 좀 해보고
하던 차에 리팩토링 이란 글을 읽었습니다. 뭐 좋은 거 이겠지요? 차차 정리해 나가보겠습니다.
리팩토링
때때로 프로그래머는 코딩을 하며 기쁘기도 하고 슬프기도 하다. 슬플때는 내 마음대로 코딩이 되지 않을 때이며 기쁠때는 내가 원하는
바대로 코딩이 되어 질 때이다. 이렇듯 코드를 만들어가며 프로그래머는 웃기도 울기도 하는 것이다.
하지만 이제 난 항상 웃고 싶어졌다.
리펙토링이란 기존의 코드가 지니고 있는 역할은 그대로 두고 코드를 수정, 재구성 하는것이다.
리팩토링은 코드를 관찰하는 것이다. 올바른 방향으로 가기 위한 끊임없는 프로그래머의 도전정신이라고도 할 수 있다. 내 몸에 꼭 맞는 코드를
만들기 위해서 노력하는 것이 바로 리팩토링이라고 할 수 있겠다.
안좋은 코드의 징후
Bad Smell은 말 그대로 나쁜 냄새를 뜻한다. 리팩토링을 아는 프로그래머들은 간혹 코드를 보며 Bad smell이 난다고 얘기한다.
그것은 그 부분이 마음에 들지 않는다는 이야기이며 리팩토링을 하고 싶다는 이야기이다. 여기서는 리팩토링을 할 만한 나쁜냄새(징후)에 대해서
몇가지 얘기해 볼까 한다.
- duplicated code
가장 심각한 냄새로 뽑히는 것중 하나로 코드의 중복을 든다. 코드의 중복이라고 하면 보통 copy and paste를 생각하겠지만,
duplication은 copy and paste만이 아닌 더 포괄적인 의미를 갖는다.
중복이라 함은 똑같은 내용이 반복적으로 사용되었슴을 말한다. 그 부분을 수정해야 할 일이 생겼을 때 똑같이 적용된 모든 부분을 찾아 다니며
수정해 주어야 하고 이에는 헛된 시간과 노력, 에러가 남는다.
- Long method
아주 긴 메써드 메써드가 길다는 의미는 라인이 길다는 의미와는 조금 다르다. 한 메써드에서 하나의 일을 처리하는 것이 아니라 여러개의 일을
한꺼번에 처리하는 경우 라인이 길 수 있다. 즉 한 메써드명이 나타내는 것만큼의 일을 해야 한다는 말이다.
보통 Long method의 경우가 duplication code를 만드는 주범이 되곤 한다. method내에 동일한 일을 하는 많은
구문들이 다른 매써드에도 존재한다면 그 것은 duplication코드이다.
이럴 경우 메써드를 잘게 나누는 방법(extract method)을 이용하여 리팩토링을 해야 한다.
- Long Parameter List
긴 파라미터 리스트.
C와 같은 순차적인 프로그래밍에 익숙한 사람들은 OOP적인 프로그래밍에 익숙치 않고 주로 메써드에 정보를 전달할 때 필요한 정보들 모두를
파라미터로 전달하려고 한다. 하지만 그 메써드를 사용하기 위해서 그 많은 파라미터를 항상 사용해야 한다면 코드를 만드는 사람도 사용하는 사람도
매우 힘들 것이다.
- Switch and long if else statement
Switch와 많은 if else 문들 역시 Bad Smell이라 할 만하다.
OOP에는 많은 개념들이 존재하는데 이 중에서도 Polymophism이라는 것이 있다. 비슷한 형태이나 행동하는 양식이 다를 경우에 이런
개념들이 사용되는데 이것은 switch case문 같은 Type에 의존적인 구문들을 대치한다.
- Comments
주석은 Bad smell?
우리는 보통 프로그래밍을 처음 배울때 주석의 중요함을 강요받는다. 자신이 만든 코드에 주석을 꼭 달아라. 귀에 못이 박히도록 말이다.
하지만 자신이 만든 코드를 이해하기 위해서 주석이 꼭 필요한 정보라면 그것은 Bad smell이라고 할 만하다. 주석이 없으면 그 코드를
이해하기 어렵다는 것은 코드가 난해하게 작성되어져 있다는 말과 동일하다 할 것이다.
변수명과 메써드명 클래스명만 가지고도 이 코드가 무슨일을 하는 코드인지 알 수 없다면 Bad smell인 것이다.
리팩토링 연습
리팩토링은 코딩을 하며 자연스럽게 알게 되는 것이긴 하나, 시작도 해 보지 않은 사람에게는 생소한 것일 수 있다. 여기서는 몇가지 기본적인
리팩토링을 예로들어 그 필요성을 증명하고자 한다. 이에 감흥을 얻게 된다면 본격적인 리팩토링을 공부해 보기를 당부한다.
다음의 코드를 구경하자.
public class Charge {
public static final int BUS = 0;
public static final int TAXI = 1;
public static final int SUBWAY = 2;
public double calculate(int type, int age, int killometer) {
double result = 0;
switch(type) {
case (BUS) :
result = calculateBus(age, killometer);
break;
case (TAXI):
result = calculateTaxi(age, killometer);
break;
case (SUBWAY):
result = calculateSubway(age, killometer);
break;
default :
throw new RuntimeException("Such type does not exist");
}
return result;
}
public double calculateBus(int age, int kilometer) {
double baseBusCharge = 600;
if(age < 15 ) {
baseBusCharge = baseBusCharge * 0.5;
}else if(age > 60) {
baseBusCharge = baseBusCharge * 0.7;
}
return baseBusCharge;
}
public double calculateTaxi(int age, int kilometer) {
double baseTaxiCharge = 3000;
return baseTaxiCharge * kilometer;
}
public double calculateSubway(int age, int kilometer) {
double baseSubwayCharge = 1000;
if (age < 15) {
baseSubwayCharge = baseSubwayCharge * 0.5;
}else if(age > 60) {
baseSubwayCharge = baseSubwayCharge * 0.7;
}
if(kilometer > 50) {
return baseSubwayCharge * 1.5;
}else {
return baseSubwayCharge;
}
}
}
나이와 거리별로 버스, 택시, 지하철의 요금을 계산해 주는 코드이다. 이 코드는 그리 심각하지는 않지만 Bad smell이 난다.
느껴지는가?
이 코드에는 type에 의한 switch-case와 비슷한 형태의 calculate메써드들이 있다. 음 어디서 부터 시작해야 할까?
리팩토링을 진행하기 전에 난 테스트 코드를 먼저 작성하기로 한다. 난 위의 코드를 완벽하게 이해하기 위해서 또한 리팩토링을 진행하기 위해서
다음과 같은 100% 통과하는 테스트 코드를 작성했다.
import junit.framework.TestCase;
public class ChargeTest extends TestCase {
public ChargeTest(String arg0) {
super(arg0);
}
public void testBusCharge() {
Charge charge = new Charge();
assertEquals(0.001, 600.0, charge.calculate(Charge.BUS, 27, 1));
assertEquals(0.001, 300.0, charge.calculate(Charge.BUS, 14, 1));
assertEquals(0.001, 420.0, charge.calculate(Charge.BUS, 70, 1));
}
public void testTaxiCharge() {
Charge charge = new Charge();
assertEquals(0.001, 6000.0, charge.calculate(Charge.TAXI, 27, 2));
assertEquals(0.001, 3000.0, charge.calculate(Charge.TAXI, 14, 1));
assertEquals(0.001, 9000.0, charge.calculate(Charge.TAXI, 70, 3));
}
public void testSubwayCharge() {
Charge charge = new Charge();
assertEquals(0.001, 1500.0, charge.calculate(Charge.SUBWAY, 27, 60));
assertEquals(0.001, 500.0, charge.calculate(Charge.SUBWAY, 14, 10));
assertEquals(0.001, 700.0, charge.calculate(Charge.SUBWAY, 70, 30));
}
}
테스트 코드는 그 의도를 잘 나타내 주고 있다고 생각한다. 가장 처음의 예를 보면 27살이고 1km거리를 버스로 이용할때의 요금은
600원인지 조사한다. 0.001이라는 수치는 double형의 결과값을 비교할때 오차범위이다.
가장 먼저 고치고 싶은 부분은 switch-case문이다.
먼저 커다란 이 클래스를 조금 잘게 나누어 보도록 하자. 우선 BusCharge라는 클래스를 만들어 보자.
BusCharge
public class BusCharge {
public double calculateBus(int age, int kilometer) {
double baseBusCharge = 600;
if (age < 15) {
baseBusCharge = baseBusCharge * 0.5;
} else if (age > 60) {
baseBusCharge = baseBusCharge * 0.7;
}
return baseBusCharge;
}
}
Charge클래스에 있던 calculateBus메써드를 그대로 BusCharge클래스로 Copy하였다. 그 이유는 caculateBus라는
메써드는 BusCharge클래스에 있는 것이 더 의미있는 것이기 때문이다. 그런후에 Charge클래스의 caculate메써드를 다음과 같이
수정한다.
public double calculate(int type, int age, int killometer) {
double result = 0;
switch(type) {
case (BUS) :
BusCharge charge = new BusCharge();
result = charge.calculateBus(age, killometer);
break;
case (TAXI):
result = calculateTaxi(age, killometer);
break;
case (SUBWAY):
result = calculateSubway(age, killometer);
break;
default :
throw new RuntimeException("Such type not exist");
}
return result;
}
Charge클래스의 caculate메써드가 caculateBus메써드를 사용하지 않고 BusCharge의 caculateBus라는 메써드를
이용하도록 바꾸어 주었다. 그리고 테스트 코드를 실행시켜 본다. 만일 테스트가 통과되지 않는다면 무엇이 잘못되었는지 금방 알 수 있을 것이다.
테스트가 통과 된다면 이제 Charge클래스의 caculateBus메써드는 더이상 사용되지 않으므로 제거한다.
이와 같은 방법으로 TaxiCharge와 SubwayCharge클래스를 만들고 각각의 caculateTaxi,
caculateSubway메써드를 Copy하고 Charge클래스의 caculate메써드를 모두 해당 클래스의 메써드를 사용하는 것으로 바꾼다.
지금까지의 결과는 다음과 같다.
Charge
public class Charge {
public static final int BUS = 0;
public static final int TAXI = 1;
public static final int SUBWAY = 2;
public double calculate(int type, int age, int killometer) {
double result = 0;
switch (type) {
case (BUS) :
BusCharge busCharge = new BusCharge();
result = busCharge.calculateBus(age, killometer);
break;
case (TAXI) :
TaxiCharge taxiCharge = new TaxiCharge();
result = taxiCharge.calculateTaxi(age, killometer);
break;
case (SUBWAY) :
SubwayCharge subwayCharge = new SubwayCharge();
result = subwayCharge.calculateSubway(age, killometer);
break;
default :
throw new RuntimeException("Such type not exist");
}
return result;
}
}
BusCharge
public class BusCharge {
public double calculateBus(int age, int kilometer) {
double baseBusCharge = 600;
if (age < 15) {
baseBusCharge = baseBusCharge * 0.5;
} else if (age > 60) {
baseBusCharge = baseBusCharge * 0.7;
}
return baseBusCharge;
}
}
TaxiCharge
public class TaxiCharge {
public double calculateTaxi(int age, int kilometer) {
double baseTaxiCharge = 3000;
return baseTaxiCharge * kilometer;
}
}
SubwayCharge
public class SubwayCharge {
public double calculateSubway(int age, int kilometer) {
double baseSubwayCharge = 1000;
if (age < 15) {
baseSubwayCharge = baseSubwayCharge * 0.5;
} else if (age > 60) {
baseSubwayCharge = baseSubwayCharge * 0.7;
}
if (kilometer > 50) {
return baseSubwayCharge * 1.5;
} else {
return baseSubwayCharge;
}
}
}
Charge클래스를 각각 BusCharge, TaxiCharge, SubwayCharge로 나누고 메써드를 move했을 뿐이다. 그 이유는
Charge가 모든 교통수단을 관리해야 하던 책임을 각각의 교통수단으로 책임을 전가하고 싶었기 때문이다.
이제 XXXCharge클래스 각각의 caculateBus, caculateTaxi, caculateSubway메써드는 모두 동일한
의미이므로 모두 caculate라는 이름으로 rename한다.
이제 BusCharge, TaxiCharge, SubwayCharge클래스 모두가 calculate라는 메써드를 가지고 있으므로 우리는
Chargable이라는 인터페이스를 생성하자. 구현해야 할 메써드는 당연히 calculate이다.
Chargable
public interface Chargable {
double calculate(int age, int kilometer);
}
그리고 각각의 클래스(BusCharge, TaxiCharge, SubwayCharge)는 Chargable인터페이스를
implements하도록 하자. 이상없이 컴파일되고 테스트가 통과된다면 다음을 계속 진행하도록 하자.
이제 어느정도의 팩토링이 진행되었으므로 본격적으로 Charge클래스를 공격해 보도록 하자. 우선 Charge클래스에 create라는
Factory메써드를 생성하자. 모두 Chargable이라는 인터페이스를 구현했으므로 리턴 타입은 Chargable로 할 수 있다.
public class Charge {
public static final int BUS = 0;
public static final int TAXI = 1;
public static final int SUBWAY = 2;
public double calculate(int type, int age, int kilometer) {
return create(type).calculate(age, kilometer);
}
public static Chargable create(int type) {
switch (type) {
case (BUS) :
return new BusCharge();
case (TAXI) :
return new TaxiCharge();
case (SUBWAY) :
return new SubwayCharge();
default :
throw new RuntimeException("Such type not exist");
}
}
}
이제 static 필드인 BUS, TAXI, SUBWAY를 해당 클래스로 move한다. 이제 Charge클래스는 다음과 같이 변했다.
public class Charge {
public static double calculate(int type, int age, int kilometer) {
return create(type).calculate(age, kilometer);
}
public static Chargable create(int type) {
switch (type) {
case (BusCharge.BUS) :
return new BusCharge();
case (TaxiCharge.TAXI) :
return new TaxiCharge();
case (SubwayCharge.SUBWAY) :
return new SubwayCharge();
default :
throw new RuntimeException("Such type not exist");
}
}
}
테스트 코드역시 Charge.BUS로 사용했던 것을 BusCharge.BUS로 모두 수정해 준다. (TaxiCharge.TAXI,
SubwayCharge.SUBWAY등으로..) 그리고 이제 모두 static매써드이므로 테스트 코드에서 Charge클래스의 인스턴스를 만들
필요가 없어졌다.
테스트 코드는 다음과 같이 변한다.
import junit.framework.TestCase;
public class ChargeTest extends TestCase {
public ChargeTest(String arg0) {
super(arg0);
}
public void testBusCharge() {
assertEquals(0.001, 600.0, Charge.calculate(BusCharge.BUS, 27, 1));
assertEquals(0.001, 300.0, Charge.calculate(BusCharge.BUS, 14, 1));
assertEquals(0.001, 420.0, Charge.calculate(BusCharge.BUS, 70, 1));
}
public void testTaxiCharge() {
assertEquals(0.001, 6000.0, Charge.calculate(TaxiCharge.TAXI, 27, 2));
assertEquals(0.001, 3000.0, Charge.calculate(TaxiCharge.TAXI, 14, 1));
assertEquals(0.001, 9000.0, Charge.calculate(TaxiCharge.TAXI, 70, 3));
}
public void testSubwayCharge() {
assertEquals(0.001, 1500.0, Charge.calculate(SubwayCharge.SUBWAY, 27, 60));
assertEquals(0.001, 500.0, Charge.calculate(SubwayCharge.SUBWAY, 14, 10));
assertEquals(0.001, 700.0, Charge.calculate(SubwayCharge.SUBWAY, 70, 30));
}
}
또한 switch-case문을 유심히 관찰해 보니 BusCharge나 TaxiCharge, SubwayCharge라는 이름 역시
duplicate되었음을 알 수 있다. 이것을 이용해서 좀 더 유연하게 코드를 수정할 수 있는 방법으로 Class.forName을 이용해 본다.
(Class.forName은 코딩시 프로그래머가 실수할 확률이 높기는 하다.)
다음과 같이 Charge클래스에 두개의 메써드를 추가하자. 두개의 메써드는 int형 타입에 의한 개별 클래스 분기를 스트링형태의 클래스
분기로 바꾸어 주기 위한 것이다. 각각의 XXXCharge클래스의 이름이 XXX + Charge의 형태로만 구현된다면 아래의 리팩토링은 매우
효율적이게 될 것이다.
public static double calculate(String type, int age, int kilometer) {
return create(type).calculate(age, kilometer);
}
public static Chargable create(String type) {
try {
return (Chargable) Class.forName(type+"Charge").newInstance();
}catch(Exception e) {
System.err.println(e.toString());
}
return null;
}
그리고 테스트 코드에서 create(BusCharge.BUS)대신에 create(“Bus”)를 이용하는 방식으로 바꾸어 보자.
assertEquals(0.001, 600.0, Charge.calculate("Bus", 27, 1));
assertEquals(0.001, 300.0, Charge.calculate("Bus", 14, 1));
assertEquals(0.001, 420.0, Charge.calculate("Bus", 70, 1));
테스트가 통과되면 테스트 코드의 Taxi, Subway마저 모두 String형태의 값으로 바꾸고 Charge클래스에서 사용하던 int
type형태의 메써드를 모두 삭제하도록 하자.
이제 필드들 BusCharge.BUS, TaxiCharge.TAXI, SubwayCharge.SUBWAY필드값들도 필요가 없어졌으므로
삭제하도록 하자.
이쯤되면 굵직한 리팩토링은 끝난 셈이다.
이제 BusCharge, TaxiCharge, SubwayCharge클래스를 관찰해 보자.
나이에 따른 할인율은 동일한데 BusCharge와 SubwayCharge에 중복이 발견된다. 이에 우리는 나이에 따른 할인율을 담당하는
클래스를 생각해 보자. 나이에 따른 할인율을 결정하는 Age클래스를 다음과 같이 만들어 보자.
public class Age {
public static double getDiscountRate(int age) {
if (age < 15) {
return 0.5;
} else if (age > 60) {
return 0.7;
} else {
return 1.0;
}
}
}
그러면 Age클래스를 이용하여 BusCharge의 메써드를 다음과 같이 바꿀 수 있다.
public class BusCharge implements Chargable{
public double calculate(int age, int kilometer) {
double baseBusCharge = 600;
baseBusCharge = baseBusCharge * Age.getDiscountRate(age);
return baseBusCharge;
}
}
위의 메써드 역시 baseBusCharge라는 필드가 중복적으로 사용되었으므로 다음과 같이 수정한다.
public class BusCharge implements Chargable {
public double calculate(int age, int kilometer) {
return getBaseCharge() * Age.getDiscountRate(age);
}
public double getBaseCharge() {
return 600;
}
}
위와 같은 방법으로 TaxiCharge와 SubwayCharge를 변경하니 다음과 같이 되었다.
TaxiCharge
public class TaxiCharge implements Chargable {
public double calculate(int age, int kilometer) {
return getBaseCharge() * kilometer;
}
public double getBaseCharge() {
return 3000.0;
}
}
SubwayCharge
public class SubwayCharge implements Chargable {
public double calculate(int age, int kilometer) {
if(kilometer > 50) {
return getBaseCharge() * Age.getDiscountRate(age) * 1.5;
}else {
return getBaseCharge() * Age.getDiscountRate(age);
}
}
public double getBaseCharge() {
return 1000.0;
}
}
getBaseCharge라는 메써드가 모두 공통적으로 사용되었으므로 Chargable인터페이스에 getBaseCharge라는 메써드를
추가하는 것이 합당할 듯 하다.
Chargable
public interface Chargable {
double calculate(int age, int kilometer);
double getBaseCharge();
}
이상과 같이 하여 Bad smell이 느껴지는 부분에 대해서 리팩토링을 진행하였다.
리팩토링의 결과 Charge라는 하나의 클래스가 가지고 있는 책임을 여러개로 분리하고 그에 합당한 클래스를 새로 만들게 되었다. 이제
AirplaneCharge, HorseCharge등 여러개가 추가되더라도 우리는 Chargable인터페이스만 구현하는 새로운 클래스를 만들기만
하면 된다.
리팩토링 전의 코드
Charge
public class Charge {
public static final int BUS = 0;
public static final int TAXI = 1;
public static final int SUBWAY = 2;
public double calculate(int type, int age, int killometer) {
double result = 0;
switch(type) {
case (BUS) :
result = calculateBus(age, killometer);
break;
case (TAXI):
result = calculateTaxi(age, killometer);
break;
case (SUBWAY):
result = calculateSubway(age, killometer);
break;
default :
throw new RuntimeException("Such type does not exist");
}
return result;
}
public double calculateBus(int age, int kilometer) {
double baseBusCharge = 600;
if(age < 15 ) {
baseBusCharge = baseBusCharge * 0.5;
}else if(age > 60) {
baseBusCharge = baseBusCharge * 0.7;
}
return baseBusCharge;
}
public double calculateTaxi(int age, int kilometer) {
double baseTaxiCharge = 3000;
return baseTaxiCharge * kilometer;
}
public double calculateSubway(int age, int kilometer) {
double baseSubwayCharge = 1000;
if (age < 15) {
baseSubwayCharge = baseSubwayCharge * 0.5;
}else if(age > 60) {
baseSubwayCharge = baseSubwayCharge * 0.7;
}
if(kilometer > 50) {
return baseSubwayCharge * 1.5;
}else {
return baseSubwayCharge;
}
}
}
리팩토링 후의 코드
ChargeTest
import junit.framework.TestCase;
public class ChargeTest extends TestCase {
public ChargeTest(String arg0) {
super(arg0);
}
public void testBusCharge() {
assertEquals(0.001, 600.0, Charge.calculate("Bus", 27, 1));
assertEquals(0.001, 300.0, Charge.calculate("Bus", 14, 1));
assertEquals(0.001, 420.0, Charge.calculate("Bus", 70, 1));
}
public void testTaxiCharge() {
assertEquals(0.001, 6000.0, Charge.calculate("Taxi", 27, 2));
assertEquals(0.001, 3000.0, Charge.calculate("Taxi", 14, 1));
assertEquals(0.001, 9000.0, Charge.calculate("Taxi", 70, 3));
}
public void testSubwayCharge() {
assertEquals(0.001, 1500.0, Charge.calculate("Subway", 27, 60));
assertEquals(0.001, 500.0, Charge.calculate("Subway", 14, 10));
assertEquals(0.001, 700.0, Charge.calculate("Subway", 70, 30));
}
}
Charge
public class Charge {
public static double calculate(String type, int age, int kilometer) {
return create(type).calculate(age, kilometer);
}
private static Chargable create(String type) {
try {
return (Chargable) Class.forName(type + "Charge").newInstance();
} catch (Exception e) {
System.err.println(e.toString());
}
return null;
}
}
Chargable
public interface Chargable {
double calculate(int age, int kilometer);
double getBaseCharge();
}
BusCharge
public class BusCharge implements Chargable {
public double calculate(int age, int kilometer) {
return getBaseCharge() * Age.getDiscountRate(age);
}
public double getBaseCharge() {
return 600;
}
}
TaxiCharge
public class TaxiCharge implements Chargable {
public double calculate(int age, int kilometer) {
return getBaseCharge() * kilometer;
}
public double getBaseCharge() {
return 3000.0;
}
}
SubwayCharge
public class SubwayCharge implements Chargable {
public double calculate(int age, int kilometer) {
if(kilometer > 50) {
return getBaseCharge() * Age.getDiscountRate(age) * 1.5;
}else {
return getBaseCharge() * Age.getDiscountRate(age);
}
}
public double getBaseCharge() {
return 1000.0;
}
}
Age
public class Age {
public static double getDiscountRate(int age) {
if (age < 15) {
return 0.5;
} else if (age > 60) {
return 0.7;
} else {
return 1.0;
}
}
}
Refactoring Again
* 나이별 할인율은 각 교통수단마다 다를 수 있으니 Age클래스를 따로 만드는것은 바람직스럽지 않다.
* 나이별 할인율과 거리별 부가금에 대한 책임을 각 교통수단마다 가지고 있어야 한다.
이 두가지 문제점에 초점을 맞추어 리팩토링이 된 향상된 코드를 아래에 소개한다.
우선 BusCharge에서 Age클래스를 사용한 부분을 제거하고 새로운 매써드를 만든다. 그리고 버스는 거리에 따라 부가금이 없지만 그러한
내용을 BusCharge클래스에 표시한다.
BusCharge
public class BusCharge implements Chargable {
public double calculate(int age, int kilometer) {
return getBaseCharge() * getDiscountRate(age) * getDistanceRate(kilometer);
}
public double getBaseCharge() {
return 600;
}
public double getDiscountRate(int age) {
if (age < 15) {
return 0.5;
} else if (age > 60) {
return 0.7;
} else {
return 1.0;
}
}
public double getDistanceRate(int kilometer) {
return 1;
}
}
TaxiCharge, SubwayCharge역시 동일하게 적용하면 다음과 같이 된다.
TaxiCharge
public class TaxiCharge implements Chargable {
public double calculate(int age, int kilometer) {
return getBaseCharge() * getDiscountRate(age) * getDistanceRate(kilometer);
}
public double getBaseCharge() {
return 3000.0;
}
public double getDiscountRate(int age) {
return 1;
}
public double getDistanceRate(int kilometer) {
return kilometer;
}
}
SubwayCharge
public class SubwayCharge implements Chargable {
public double calculate(int age, int kilometer) {
return getBaseCharge()
* getDiscountRate(age)
* getDistanceRate(kilometer);
}
public double getBaseCharge() {
return 1000.0;
}
public double getDiscountRate(int age) {
if (age < 15) {
return 0.5;
} else if (age > 60) {
return 0.7;
} else {
return 1.0;
}
}
public double getDistanceRate(int kilometer) {
if (kilometer > 50) {
return 1.5;
} else {
return 1;
}
}
}
그리고 각각의 클래스가 모두 공통적인 메써드인 calculate가 있으므로 상단으로 끌어올리기 위해서 다음과 같은
ChargeCalculator클래스를 생성하고 각각(BusCharge, TaxiCharge, SubwayCharge)의 calculate메써드를
삭제한다.
그리고 각각의 교통수단을 나타내는 클래스는 ChargeCalculator클래스를 extends한다.
ChargeCalculator
public abstract class ChargeCalculator implements Chargable {
public double calculate(int age, int kilometer) {
return getBaseCharge()
* getDiscountRate(age)
* getDistanceRate(kilometer);
}
}
그리고 Chargable 인터페이스는 다음과 같이 바뀐다. Chargable인터페이스는 각 교통수단 클래스가 꼭 지녀야 할 책임을 말한다.
따라서 아래와 같이 calculate메써드는 사라지고 나이별 할인율과 거리별 부가율에 대한 메써드가 추가 되었다.
Chargable
public interface Chargable {
double getBaseCharge();
double getDiscountRate(int age);
double getDistanceRate(int kilometer);
}
Charge클래스 역시 다음과 같이 바뀐다.
Charge
public class Charge {
public static double calculate(String type, int age, int kilometer) {
return create(type).calculate(age, kilometer);
}
private static ChargeCalculator create(String type) {
try {
return (ChargeCalculator) Class.forName(type + "Charge").newInstance();
} catch (Exception e) {
System.err.println(e.toString());
}
return null;
}
}
어쩌면 아직도 리팩토링할 거리가 많이 남아 있는지도 모르겠다. 그것은 독자의 몫으로 남겨 놓는다.
리팩토링 참고
본인이 강추하는 책중의 하나로 Martin Fowler가 쓴 Refactoring이라는 책을 든다. 경험에서 우러나오는 감동적인 글이다.
Bad smell에 대한 정의와 그 해결방법을 체계적으로 들고 있다.
혹자는 이 책은 프로그래머의 내공을 약 3배정도 증가시켜준다고 한다.
- http://www.refactoring.com – 리팩토링 공식 사이트