Clean Code 3장 - 함수

클린 코드 책을 읽으면서 해당 단원이 가장 쉬운 부분 같지만 잘 지키지 않고, 그리고 지키면 가장 효과가 가장 크다 생각한다.

 

아래 함수를 보면 이해가 잘 되는가? 해당 함수를 깨끗하게 만들어보자.

 

public static String renderPageWithSetupsAndTeardowns( PageData pageData, boolean isSuite) throws Exception {
	boolean isTestPage = pageData.hasAttribute("Test"); 
	if (isTestPage) {
		WikiPage testPage = pageData.getWikiPage(); 
		StringBuffer newPageContent = new StringBuffer(); 
		includeSetupPages(testPage, newPageContent, isSuite); 
		newPageContent.append(pageData.getContent()); 
		includeTeardownPages(testPage, newPageContent, isSuite); 
		pageData.setContent(newPageContent.toString());
	}
	return pageData.getHtml(); 
}

함수를 작게 만들어라! & if 블록에는 한줄만!

public static String renderPageWithSetupsAndTeardowns( PageData pageData, boolean isSuite) throws Exception { 
   if (isTestPage(pageData)) 
   	includeSetupAndTeardownPages(pageData, isSuite); 
   return pageData.getHtml();
}

훨씬 읽기가 좋아졌다. if 문안에서는 특정 행동을 할텐데 이 부분을 함수로 빼자. 그렇게 되면 코드를 읽을 때 훨씬 쉬워진다.

한 가지 일만 해라!

함수는 한 가지만의 일을 해야 한다. 그렇다면 그 한가지만의 일을 하는지 아닌지 판단은 어떻게 할 것인가?

• 맨 처음 예제 코드를 보면 추상화 수준이 둘이다. 밑에 코드처럼 축소할 수 있다면 한가지 일을 하고 있는 것이 아니다

함수당 추상화 수준은 하나로!

만약 한 함수내에 추상화 수준이 섞이게 되면 읽는 사람이 헷갈린다.

 

위에서 아래로 읽을 수 있도록 짜자

코드는 기사고 이야기이다. 그냥 읽는대로 읽혀야 깔끔한 코드다!

Switch 문

public Money calculatePay(Employee e) throws InvalidEmployeeType {
	switch (e.type) { 
		case COMMISSIONED:
			return calculateCommissionedPay(e); 
		case HOURLY:
			return calculateHourlyPay(e); 
		case SALARIED:
			return calculateSalariedPay(e); 
		default:
			throw new InvalidEmployeeType(e.type); 
	}
}

해당 코드는 무슨 문제가 있을까?

• 함수가 길다! -> 직원 유형이 추가되면 더 길어진다.
• '한가지' 작업만 수행하지 않는다.
• SRP(single responsibility principle) 위반 -> 코드를 변경할 이유가 많다.
• OCP(open close principle) 위반 -> 직원 유형이 추가될 때마다 코드를 변경해야 한다.
• 가장 심각한 문제! 다른 종류의 함수 isPayDay(Employee e, Date date) 와 같은 것이 있으면 무한 반복이다...

해당 코드를 리팩토링하면 아래와 같이 짤 수 있다.

public abstract class Employee {
	public abstract boolean isPayday();
	public abstract Money calculatePay();
	public abstract void deliverPay(Money pay);
}
-----------------
public interface EmployeeFactory {
	public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType; 
}
-----------------
public class EmployeeFactoryImpl implements EmployeeFactory {
	public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType {
		switch (r.type) {
			case COMMISSIONED:
				return new CommissionedEmployee(r) ;
			case HOURLY:
				return new HourlyEmployee(r);
			case SALARIED:
				return new SalariedEmploye(r);
			default:
				throw new InvalidEmployeeType(r.type);
		} 
	}
}

switch 문은 다형적 객체를 생성하는 코드에서만 쓰고 웬만해서는 사용하지 말자!

서술적인 이름을 사용하라!

“코드를 읽으면서 짐작했던 기능을 각 루틴이 그대로 수행한다면 깨끗한 코드라 불러도 되겠다” - 워드

함수 인수 개수는 몇개가 적당한가?

이상적인 함수인수는 0개이고, 필요하다면 1개까지이다.

• 많이 쓰는 단항 형식

boolean fileExists(“MyFile”);

InputStream fileOpen(“MyFile”);

• 플래그 인수는 쓰지마라 - boolean값을 넘기는것 자체가 그 함수는 한번에 여러가지 일을 하는 것이다
• 이항 함수도 웬만하면 단항함수로 바꾸자!

Point p = new Point(x,y); // 이정도는 이해해줄 수 있다

writeField(outputStream, name)
outputStream.writeField(name) // 클래스 구성원 변수로 만들어버리자!

• 삼항 함수는 그냥 만들지 말자!
• 인수 객체가 2-3개 필요하다면 일부를 독자적인 클래스 변수로 선언하자
  -> 단순히 눈속임이라고? 쓸데없는 짓이라고? 해당 리팩토링 덕분에 개발자는 개념을 이해하기 쉬워지고 실수가 줄어든다!

Circle makeCircle(double x, double y, double radius);
Circle makeCircle(Point center, double radius);

• 동사와 키워드

writeField(name); // 단항 함수는 인수가 동사,명사 쌍을 이뤄야 한다.
assertExpectedEqualsActual(expected, actual); // 함수 이름에 키워드를 넣으면 순서를 기억할 필요가 없다

부수 효과를 일으키지 말자!

검증을 하겠다는 함수에서 남몰래 Session.initialize()를 하고 있다! 이런 함수명과 맞지 않는 부수효과를 내서는 안된다!

public class UserValidator {
	private Cryptographer cryptographer;
	public boolean checkPassword(String userName, String password) { 
		User user = UserGateway.findByName(userName);
		if (user != User.NULL) {
			String codedPhrase = user.getPhraseEncodedByPassword(); 
			String phrase = cryptographer.decrypt(codedPhrase, password); 
			if ("Valid Password".equals(phrase)) {
				Session.initialize();
				return true; 
			}
		}
		return false; 
	}
}

출력인수는 피하자!

함수에서 상태를 변경해야 한다면 함수가 속한 객체 상태를 변경하는 방식을 사용하자!

오류코드보다 예외를 사용하라!

if (deletePage(page) == E_OK) {
	if (registry.deleteReference(page.name) == E_OK) {
		if (configKeys.deleteKey(page.name.makeKey()) == E_OK) {
			logger.log("page deleted");
		} else {
			logger.log("configKey not deleted");
		}
	} else {
		logger.log("deleteReference from registry failed"); 
	} 
} else {
	logger.log("delete failed"); return E_ERROR;
}

public void delete(Page page) {
	try {
		deletePageAndAllReferences(page);
  	} catch (Exception e) {
  		logError(e);
  	}
}

private void deletePageAndAllReferences(Page page) throws Exception { 
	deletePage(page);
	registry.deleteReference(page.name); 
	configKeys.deleteKey(page.name.makeKey());
}

private void logError(Exception e) { 
	logger.log(e.getMessage());
}