<리팩토링> 소스코드 리팩토링 기본 개념, 두번째

 

 

임시변수 없애기

임시변수는 자체 루틴 안에서만 효력이 있다보니 점점 더 많은 임시변수를 사용하게 되어 코드가 복잡해지게 된다. 현재 임시변수는 두개가 있으며 임시변수를 메서드 호출로 전환 기법을 사용해서 질의 메서드로 고친다. 질의 메서드는 클래스 안에 모든 메서드에서 접근 가능하므로 메서드를 복잡하게 만드는 이런 임시변수를 사용하지 않아도 되니 설계가 깔끔해진다.

class Customer {
…
		// 푸터행 추가
		result += "누적된 대여료 : " + String.valueOf(getTotalCharge()) + "\n";
		result += "적립 포인트 : " + String.valueOf(getTotalfrequentRenterPoints())
				+ "\n";
		return result;	
}

	private double getTotalCharge(){
		double result = 0;
		Enumeration rentals = _rentals.elements();
		while(rentals.hasMoreElements()){
			Rental each = (Rental) rentals.nextElement();
			result += each.getCharge();
		}
		return result;
	}
	private int getTotalfrequentRenterPoints() {
		int result = 0;
		Enumeration rentals = _rentals.elements();
		while (rentals.hasMoreElements()) {
			Rental each = (Rental) rentals.nextElement();
			result += each.getFrequentRenterPoints();
		}
		return result;
	}
…

 

이 리팩토링의 경우 한가지 문제점은 성능이다. 수정 전 코드는 while문 루프가 1회만 실행했는데 수정 후 코드는 3회나 실행한다. 오랜 시간이 거리는 while문으로 인해 성능이 저하된다. 이러한 이유로 리팩토링을 하려 하지 않으려 하지만 항상 다양한 경우의 수를 생각하자.

while문은 최적화 단계에서 걱정해도 늦지않다. 최적화 단계가 성능 해결의 적기이며 효과적인 최적화를 위한 더 많은 선택의 여지가 있다.

---

HTML statement작성

계산 부분을 빼내서 htmlStatement메서드로 작성하면 처음의 statement메서드에 들어있는 계산 코드를 전부 재사용 할 수 있다.

 

public String htmlStatement(){
		Enumeration rentals = _rentals.elements();
		String result = "<H1><EM>"+getName() + "고객님 대여기록</EM></H1><P>\n";
		while (rentals.hasMoreElements()) {
			Rental each = (Rental) rentals.nextElement();

			// 이번에 대여하는 비디오와 정보, 대여료 출력
			result +=  each.get_movie().get_title() + " : " + 
					+ (each.getCharge()) + "<BR>\n";

		}
		// 푸터행 추가
		result += "<P>누적된 대여료 : <EM>" + String.valueOf(getTotalCharge()) + "</EM><P>\n";
		result += "적립 포인트 : <EM>" + String.valueOf(getTotalfrequentRenterPoints()+"</EM><P>")
				+ "\n";
		return result;
	}

 

 

 

 

가격 책정 부분의 조건문을 재정의로 교체

제일 먼저 고칠 부분은 switch문이다. 타 객체의 속성을 swtich문의 인자로 하는 것은 나쁜 방법이다. switch문의 인자로는 타 객체 데이터를 사용하지 말고 자신의 데이터를 사용해야한다.

따라서 getCharge 메서드를 Movie클래스로 옮겨야한다.

 

public class Movie {
…
	public double getCharge(int daysRented) {
		int result = 0;
		switch (get_priceCode()) {
		case Movie.REGULAR:
			result += 2;
			if (daysRented > 2) {
				result += (daysRented- 2) * 1.5;
			}
			break;
		case Movie.NEW_RELEASE:
			result += (daysRented) * 3;
			break;
		case Movie.CHILDRENS:
			result += 1.5;
			if (daysRented > 3) {
				result += (daysRented- 3) * 1.5;
			}
			break;
		}
		return result;
	}

	public int getFrequentRenterPoints(int daysRented) {
		// 최신물 2틀이상 대여 : 2포인트
		// 그외 1포인트 지급
		if (get_priceCode() == Movie.NEW_RELEASE
				&& daysRented > 1)
			return 2;
		else
			return 1;
	}
class Rental {
…
	public double getCharge(int daysRented) {
		return _movie.getCharge(daysRented);
		
	}
	public int getFrequentRenterPoints() {
			return _movie.getFrequentRenterPoints(_daysRented);
	}

 

 

 

 

 

상속구조만들기

Movie클래스는 비디오 종류에 따라 같은 메서드 호출에도 각기 다른 값을 반환한다. 그런데 이건 하위 클래스가 처리할 일이다. 따라서 Movie클래스를 상속받는 3개의 하위 클래스를 작성하고 비디오 종류별 대여료 계산을 각 하위 클래스에 넣어야 한다.

하위 클래스로 작성해 상속구조로 만들면 switch문을 재정의로 바꿀 수 있다. 하지만 수명주기동안 비디오는 언제든 분류가 바뀔 수 있기 때문에 4인방의 상태 패턴을 적용해 switch문을 삭제한다.

인다이렉션 기능을 추가하면 Price클래스 안의 코드를 하위 클래스로 만들어서 언제든 대여료를 변경할 수 있다.

 

//리팩토링된 전체 코드
package com.pnpsecure.chap1;

import java.util.Enumeration;
import java.util.Vector;

public class Movie {
	public static final int CHILDRENS = 2;
	public static final int REGULAR = 0;
	public static final int NEW_RELEASE = 1;
	private String _title;
	private Price _price;

	public Movie(String title, int priceCode) {
		_title = title;
		set_priceCode(priceCode);
	}
	public double getCharge(int dayRented) {
		return _price.getCharge(dayRented);
	}

	public int get_priceCode() {
		return _price.getPriceCode();
	}

	public void set_priceCode(int args) {
		switch (args) {
		case REGULAR:
			_price = new RegularPrice();
			break;
		case NEW_RELEASE:
			_price = new NewReleasePrice();
			break;
		case CHILDRENS:
			_price = new ChildrensPrice();
			break;
		default:
			throw new IllegalArgumentException("가격 코드가 잘못 되었습니다.");
		}
	}

	public String get_title() {
		return _title;
	}

	public int getFrequentRenterPoints(int daysRented) {
		return _price.getFrequentRenterPoints(daysRented);
	}

	abstract class Price {
		abstract int getPriceCode();

		abstract double getCharge(int dayRented);

		public int getFrequentRenterPoints(int daysRented) {
			return 1;
		}
	}

	class ChildrensPrice extends Price {
		int getPriceCode() {
			return Movie.CHILDRENS;
		}

		public double getCharge(int daysRented) {
			double result = 1.5;
			if (daysRented > 3) {
				result += (daysRented - 3) * 1.5;
			}
			return result;
		}
	}

	class NewReleasePrice extends Price {
		int getPriceCode() {
			return Movie.NEW_RELEASE;
		}

		public double getCharge(int daysRented) {
			return daysRented * 3;
		}
		public int getFrequentRenterPoints(int daysRented) {
			return (daysRented > 1) ? 2 : 1;
		}
	}

	class RegularPrice extends Price {
		int getPriceCode() {
			return Movie.REGULAR;
		}

		public double getCharge(int daysRented) {
			double result = 2;
			if (daysRented > 2) {
				result += (daysRented - 2) * 1.5;
			}
			return result;
		}
	}

}

class Rental {
	private Movie _movie;
	private int _daysRented;

	public Rental(Movie movie, int daysRented) {
		// TODO Auto-generated constructor stub
		_movie = movie;
		_daysRented = daysRented;
	}

	public int get_daysRented() {
		return _daysRented;
	}

	public Movie get_movie() {
		return _movie;
	}

	public double getCharge() {
		return _movie.getCharge(_daysRented);

	}

	public int getFrequentRenterPoints() {
		return _movie.getFrequentRenterPoints(_daysRented);
	}
}

class Customer {
	private String _name;
	private Vector _rentals = new Vector();

	public Customer(String name) {
		// TODO Auto-generated constructor stub
		_name = name;
	}

	public void addRental(Rental arg) {
		_rentals.addElement(arg);
	}

	public String getName() {
		return _name;
	}

	public String statement() {
		Enumeration rentals = _rentals.elements();
		String result = getName() + "고객님 대여기록\n";
		while (rentals.hasMoreElements()) {
			Rental each = (Rental) rentals.nextElement();

			// 이번에 대여하는 비디오와 정보, 대여료 출력
			result += "\t" + each.get_movie().get_title() + "\t"
					+ (each.getCharge()) + "\n";

		}

		// 푸터행 추가
		result += "누적된 대여료 : " + String.valueOf(getTotalCharge()) + "\n";
		result += "적립 포인트 : " + String.valueOf(getTotalfrequentRenterPoints())
				+ "\n";
		return result;
	}

	private int getTotalfrequentRenterPoints() {
		int result = 0;
		Enumeration rentals = _rentals.elements();
		while (rentals.hasMoreElements()) {
			Rental each = (Rental) rentals.nextElement();
			result += each.getFrequentRenterPoints();
		}
		return result;
	}

	private double getTotalCharge() {
		double result = 0;
		Enumeration rentals = _rentals.elements();
		while (rentals.hasMoreElements()) {
			Rental each = (Rental) rentals.nextElement();
			result += each.getCharge();
		}
		return result;
	}

	public String htmlStatement() {
		Enumeration rentals = _rentals.elements();
		String result = "<H1><EM>" + getName() + "고객님 대여기록</EM></H1><P>\n";
		while (rentals.hasMoreElements()) {
			Rental each = (Rental) rentals.nextElement();

			// 이번에 대여하는 비디오와 정보, 대여료 출력
			result += each.get_movie().get_title() + " : "
					+ +(each.getCharge()) + "<BR>\n";

		}
		// 푸터행 추가
		result += "<P>누적된 대여료 : <EM>" + String.valueOf(getTotalCharge())
				+ "</EM><P>\n";
		result += "적립 포인트 : <EM>"
				+ String.valueOf(getTotalfrequentRenterPoints() + "</EM><P>")
				+ "\n";
		return result;
	}

}

 

과연 이렇게 해서 적용할 가치가 있을까? 지금까지 다룬 예제의 기능이 별로 없을 땐 이 장점이 미미해 보일 수 있지만, 십여개가 넘는 대여료 관련 메서드로 구성된 복잡한 시스템이라면 무시할 수 없는 차이를 보인다. 리팩토링의 각 단계에서 실시한 수정이 미미하다 보니 조금씩 수정하는 것이 답답해 보일 수 있지만 디버거를 실행할 일도 없으니 실제 작업은 상당히 빠른 편이다.