포포핀
939
2021-07-08 09:13:32 작성 2021-07-08 09:16:54 수정됨
8
3060

JAVA 조건문 리팩토링


JAVA 조건문 리팩토링


조건문잡기

프로그램에서 읽기 어려고 복잡한 부분을 보면 주로 조건문이 많다.
이번 Archives에선 코드의 가독성을 떨어뜨리는 악의 축 조건문을 물리치는(?) 방법을 알아보기로한다.
우선 다음 막장 코드를 보자


private List<Student> students = new ArrayList<>();
private static final int BASE_TUITION = 100000;

public void process() {    
    int tuition = 0;
    
    LocalDate now = LocalDate.now();
    if(now.compareTo(LocalDate.of(now.getYear(), 07, 25)) > 0
            && now.compareTo(LocalDate.of(now.getYear(), 8, 30)) < 0) {
        System.out.println(now.format(DateTimeFormatter.BASIC_ISO_DATE));
        System.out.println("여름 방학입니다");
        
        tuition = BASE_TUITION * 2;
        
        if(hasAnyFailure()) {
            createVacationCourse();
        }

    }else if(now.compareTo(LocalDate.of(now.getYear(), 12, 20)) > 0
            && now.compareTo(LocalDate.of(now.getYear(), 2, 28)) < 0) {		
        System.out.println(now.format(DateTimeFormatter.BASIC_ISO_DATE));	
        System.out.println("겨울 방학입니다.");

        tuition = BASE_TUITION + 10000;			
        
        if(hasAnyFailure()) {
            createVacationCourse();
        }
        
    }else {
        System.out.println(now.format(DateTimeFormatter.BASIC_ISO_DATE));
        tuition = BASE_TUITION + getAddtionalTuition(now);
        createCourse();
    }
}

private boolean hasAnyFailure() {
    boolean anyFail = false;
    for(Student student : students) {
        if(student.isFail()) {
            anyFail = true;
        }        
    }

    return anyFail;
}

private int getAddtionalTuition(LocalDate startDate) {
    int result = 0;
    if(startDate.getMonth() == Month.MARCH) {
        result = 30000;
    }else {
        if(startDate.getDayOfWeek() == DayOfWeek.SUNDAY) {
            result = 20000; 
        }else {
            result = 25000;
        }
    }
    
    return result;
}

// 구현 안함
class Student {
    boolean isFail() { return true; }
    String getMajor() { return ""; }
    int getAge() { return 0; }
}


무슨 기능을 하는지 파악이 되는가? if문안에 if문 계속되는 분기문으로 이해하기가 어렵다.
물론 지금은 간단하게 만들어서 어느정도 시간을 들이면 파악이 되겠지만 시스템이 점점 복잡해지고 분기문이 계속 증가한다면 나중에는 소스분석이 불가능할 정도가 될 것이다.
위 코드의 기능을 간단히 설명하면 오늘 날짜가 여름방학, 겨울방학인지 구분해서
방학이면 학생들 가운데 낙제자가 있는지 파악하고 있으면 방학 강좌을 개설하고 학비를 계산하는 것이다.
리팩토링을 위해 억지로 만든 예제라서 조금 말이 안되지만 사실 여기서 코드가 무슨 기능을 하는 지는 중요하지 않다.
우리의 목적은 조건문을 리펙토링하는 것이다.
이제 위 코드의 리펙토링을 단계별로 진행해보겠다.

1. 조건문의 공통 코드를 빼내기

조건문마다 공통되는 코드가 들어가 있다면 해당 코드를 조건문 밖으로 빼내야한다.
막장코드에서 공통되는 코드는 다음 부분이다.

System.out.println(now.format(DateTimeFormatter.BASIC_ISO_DATE));


위 코드를 조건문 밖으로 보내면 코드는 다음과 같다.

public void process() {    
    int tuition = 0;
    
    LocalDate now = LocalDate.now();
    System.out.println(now.format(DateTimeFormatter.BASIC_ISO_DATE));   // 조건문 앞으로 뺌

    if(now.compareTo(LocalDate.of(now.getYear(), 07, 25)) > 0
            && now.compareTo(LocalDate.of(now.getYear(), 8, 30)) < 0) {
        System.out.println("여름 방학입니다");
        
        tuition = BASE_TUITION * 2;
        
        if(hasAnyFailure()) {
            createVacationCourse();
        }

    }else if(now.compareTo(LocalDate.of(now.getYear(), 12, 20)) > 0
            && now.compareTo(LocalDate.of(now.getYear(), 2, 28)) < 0) {		
        System.out.println("겨울 방학입니다.");

        tuition = BASE_TUITION + 10000;			
        
        if(hasAnyFailure()) {
            createVacationCourse();
        }
        
    }else {
        tuition = BASE_TUITION + getAddtionalTuition(now);
        createCourse();
    }
}


2.조건문을 매서드로 분리

막장코드에서 아래의 조건문은 무슨 판단을 하는 조건인지 이해하기 어렵다

if(now.compareTo(LocalDate.of(date.getYear(), 07, 25)) > 0
            && now.compareTo(LocalDate.of(date.getYear(), 8, 30)) < 0) {
    //...
}else if(now.compareTo(LocalDate.of(date.getYear(), 12, 20)) > 0
            && now.compareTo(LocalDate.of(date.getYear(), 2, 28)) < 0) {
    //...
}            


if 절을 메서드로 분리하자
변경된 코드는 아래와 같다.

if(isSummerVacation(now)) {
    //...
}else if(isWinterVacation(now)) {
    //...
}

private boolean isSummerVacation(LocalDate date) {
    return date.compareTo(LocalDate.of(date.getYear(), 07, 25)) > 0
            && date.compareTo(LocalDate.of(date.getYear(), 8, 30)) < 0;
}

private boolean isWinterVacation(LocalDate date) {
    return date.compareTo(LocalDate.of(date.getYear(), 12, 20)) > 0
            && date.compareTo(LocalDate.of(date.getYear(), 2, 28)) < 0;
}

메서드명만 잘 지으면 이름만 보고도 무엇을 판단하는 로직인지 쉽게 알 수 있다.

3.제어 플래그 제거

hasAnyFailure() 메서드를 보자. 학생중에 낙제한 학생이 있는지 찾는 기능이다.
지역변수로 anyFail 이라는 제어 플래그가 존재하는데 이런 플래그는 break, continue, return 문으로 제거가 가능하다.
변경 전 후 코드는 다음과 같다

// 변경 전
private boolean hasAnyFailure() {
    boolean anyFail = false;
    for(Student student : students) {
        if(student.isFail()) {
            anyFail = true;
        }        
    }

    return anyFail;
}

// 변경 후
private boolean hasAnyFailure() {
    for(Student student : students) {
        if(student.isFail()) {
            return true;
        }
    }
	    
    return false;
}

불필요한 지역변수 플래그가 사라지고 루프를 무조건 전부 돌 필요가 없기 때문에 성능도 향상된다.

4. 감시절로 전환

getAddtionalTuition() 메서드를 보자
특정 개강날짜의 추가된 학비를 계산하는 기능이다.
예외적인 조건을 처리하는 분기문은 감시절로 전환하면 더 명확하고 가독성 좋은 코드가 된다.

// 변경 전
private int getAddtionalTuition(LocalDate startDate) {
    int result = 0;
    if(startDate.getMonth() == Month.MARCH) {
        result = 30000;
    }else {
        if(startDate.getDayOfWeek() == DayOfWeek.SUNDAY) {
            result = 20000; 
        }else {
            result = 25000;
        }
    }
    
    return result;
}

// 변경 후
private int getAddtionalTuition(LocalDate startDate) {    
    if(startDate.getMonth() == Month.MARCH) {
        return 30000;
    }
    if(startDate.getDayOfWeek() == DayOfWeek.SUNDAY) {
        return 20000; 
    }
    return 25000;
}


5. 다형성을 이용한 재정의로 전환

process() 메서드를 보면 일반/여름방학/겨울방학 분류에 따라 다르게 동작한다.
이처럼 조건에따라 다르게 동작하는 경우는 객체의 다형성을 이용해 분리하는게 좋다.
먼저 각각의 if문 블럭을 보면 다음과 같이 하는 일이 비슷하다

  • 방학을 알린다.
  • 수업료를 계산한다.
  • 강좌를 개설한다.

위 메시지를 추상화하여 다음과 같은 추상클래스를 만든다.

public abstract class Course {
	protected List<Student> students = new ArrayList<>();
	protected static final int BASE_TUITION = 100000;
	
	abstract void notifyVacation();
	abstract int getTuition(); 
	abstract void createCourse();
	
	protected boolean hasAnyFailure() {
	    for(Student student : students) {
	        if(student.isFail()) {
	            return true;
	        }
	    }
	    return false;
	}
}

중요한 것은 3개의 추상메서드이다.

  • notifyVacation() - 방학을 알린다.
  • getTuition() - 수업료를 계산한다.
  • createCourse() - 강좌를 개설한다.

구현 클래스에서 사용이 필요한 필드(students, BASE_TUITION)와 메서드(hasAnyFailure())들도 추상클래스로 이동시켰다.
이제 각각의 구현클래스를 만들면 다음과 같다

public class SummerVactionCourse extends Course {
	@Override
	public void notifyVacation() {
		System.out.println("여름 방학입니다");
	}

	@Override
	public int getTuition() {
		return BASE_TUITION * 2;
	}
	@Override
	public void createCourse() {
		if(hasAnyFailure()) createVacationCourse();        
    }
	
	private void createVacationCourse() {
		//...
	}
}
public class WinterVacationCourse extends Course {
	@Override
	public void notifyVacation() {
		System.out.println("겨울 방학입니다.");
	}

	@Override
	public int getTuition() {
		return BASE_TUITION + 10000;
	}

	@Override
	public void createCourse() {
		if(hasAnyFailure()) createVacationCourse();
	}
	
	private void createVacationCourse() {
		//...
	}
}
public class RegularCourse extends Course {
	private LocalDate now;
	
	public RegularCourse(LocalDate date) {
		this.now = date;
	}

	@Override
	public void notifyVacation() {
	}

	@Override
	public int getTuition() {
		return BASE_TUITION + getAddtionalTuition(now);
	}

	@Override
	public void createCourse() {
		createRegularCourse();
	}
	
	private int getAddtionalTuition(LocalDate startDate) {    
	    if(startDate.getMonth() == Month.MARCH) {
	        return 30000;
	    }
	    if(startDate.getDayOfWeek() == DayOfWeek.SUNDAY) {
	        return 20000; 
	    }
	    return 25000;
	}

	private void createRegularCourse() {
		//...
	}
}


process()의 조건문을 보면 현재 날짜에 따라 Course의 구현 타입이 나뉘는 것을 알 수 있다.
날짜에 따라 해당 타입을 생성하는 팩토리 클래스를 만들어주자

public class CourseFactory {	
	public static Course CreateCourse(LocalDate date) {
		if(isSummerVacation(date)) {
			return new SummerVactionCourse();
		}else if(isWinterVacation(date)) {
			return new WinterVacationCourse();
		}else {
			return new RegularCourse(date);
		}
	}
	
	private static boolean isSummerVacation(LocalDate date) {
		return date.compareTo(LocalDate.of(date.getYear(), 07, 25)) > 0
			&& date.compareTo(LocalDate.of(date.getYear(), 8, 30)) < 0;
	}
	
	private static boolean isWinterVacation(LocalDate date) {
		return date.compareTo(LocalDate.of(date.getYear(), 12, 20)) > 0
	            && date.compareTo(LocalDate.of(date.getYear(), 2, 28)) < 0;
	}
}


이제 분리는 끝났다 아래는 지금까지 리팩토링한 소스의 UML이다.
UML

마지막으로 process() 메서드를 변경하면 다음과 같다.

public void process() {
    LocalDate now = LocalDate.now();
    System.out.println(now.format(DateTimeFormatter.BASIC_ISO_DATE));
    
    Course course = CourseFactory.CreateCourse(now);
    course.notifyVacation();
    tuition = course.getTuition();
    course.createCourse();
}

외부에서는 강좌가 어떻게 생성되는지, 수업료가 어떻게 계산되는지 알 필요가없다 (캡슐화가 잘 되었다)
다른 강좌를 추가하더라도 새로운 Course 클래스를 구현하기만하면 되므로 Course 클래스를 변경할 필요가 없다 (OCP 원칙도 잘 지켜진다)
복잡한 조건문도 없어지면서 어떤 일을 하는지 한눈에 알 수 있게되었다.

처음 코드와 비교해보면 많이 발전했다 ^^
하지만 아직 리팩토링이 완벽하진않다. 객체간 책임이동이 더 필요하고 몇가지 문제가 있지만 조건문 리팩토링의 주제에서 벗어나므로 이쯤에서 마무리한다.

  • 참조 : [책] 리팩토링 - 마틴 파울러
16
11
  • 댓글 8

  • 어쩌다프로그래머
    6k
    2021-07-08 13:39:46
    위 조건문만 보자면 enum 에 function 사용이 훨씬 간결해 집니다.
    -1
  • 포포핀
    939
    2021-07-09 09:26:34

    enum은 상수형 집합을 정의하기 위함인데 위 코드에서 Course 객체가 constant인지 생각해 보면 그렇지 않습니다. 언제든지 확장이 가능해야되고 상태값도 가지고 있습니다.

    확장가능한 다형성을 위해 enum을 사용하는 것은 enum의 목적에 부합하지 않는 과도한 사용이라고 생각합니다.

  • 여의도 한량이
    198
    2021-07-09 13:17:25
    좋은 올려주셔서 감사합니다 잘보고갑니다!!
  • 타느스
    485
    2021-07-11 13:05:50
    도메인을 분리하면 저런 전제가 안나올 것 같은데요..
  • 드루이드
    78
    2021-07-12 02:07:37 작성 2021-07-12 02:11:23 수정됨

    좋은글 감사합니다. 그런데 리팩토링 되지않은 코드를 막장이라고 하기보단 리팩토링 되지 않은 코드나 급하게 짠듯한 코드등 좀더 순한 표현이 좋지 않을까요? 물론 예제를 직접 만드셨겠지만 혹시 누군가 짜놓은 코드를 리팩토링 한다면 .... ㅋㅋ 누구나 미숙한 시기가 있고 그때 짜놓은 코드를 막장이라고 칭한다면 슬플거 같습니다. ㅎ  

  • 연습용더미1
    669
    2021-07-13 17:12:06

    4번은 오히려 보기 껄끄러워지는 기분인데... 맞는건가요?

  • 포포핀
    939
    2021-07-13 17:41:25

    드루이드

    처음엔 레거시코드라 지칭할려고했는데 임의로 만든 예제라서 레거시라고 하기도 좀 이상하고, 처음 그림처럼 조건문을 물리치는 컨셉으로 작성하다보니 다소 과한 표현이된거 같습니다 ㅎㅎ

    연습용더미1

    사실 예외적인 조건을 표현하기 위해 아래처럼 리팩토링 하는건데 예제가 예외적인 조건이냐 생각해보면 맞을 수도 있고 아닐 수도 있습니다. 예제가 좀 애매하긴하네요. 하지만 else 블럭에 긴 코드가 들어간다고 생각해보면 아래가 더 읽기 쉽다고 생각합니다.


  • Dive_Drink_Develope
    6k
    2021-07-14 18:30:00

    저도 4번  리팩토링 후가 한눈에 더 잘읽힙니다.

     아 3월 일요일 기타 그렇군

  • 로그인을 하시면 댓글을 등록할 수 있습니다.