6. 예측 가능한 코드를 작성하라

개요

1. 매직값을 반환하지 말아야 한다

매직값은 버그를 유발할 수 있다

class User {
	Int getAge() { 
		if (age == null) {
			return -1;
		}
	return age;
}

Double? getMeanAge(List<User> users) {
	if (users.isEmpty()) {
		return null;
	}
	Double sumOfAges = 0.0;
	for (user in users) {
		sumOfAges += user.getAge().toDouble()
	}
	return sumOfAges / users.size().toDouble()
}

• getMeanAge 함수를 작성하는 작성자가 -1을 반환한다는 사실을 모르고 작성하면 위와같이 버그가 발생할 수 있다.

해결책 : 널, 옵셔널 또는 오류를 반환하라

• 함수에서 매직값을 반환할 때의 문제점은 호출하는 쪽에서 함수 계약의 세부 조항을 알아야 한다는 점이다.
어떤 개발자들은 이 세부 조항을 확인하지 않거나 확인하고나서 잊어버린다. 이런 경우에는 예측을 벗어나는 일이 생길 수 있다.

• 값이 없을 수 있는 경우 이것이 코드 계약의 명백한 부분에서 확인할 수 있도록 하는 것이 훨씬 좋다. 이를 위한 쉬운 방법은
• 널 안정성을 지원하는 경우 널이 가능한 유형 반환 ,
• 널 안정성 지원하지 않는 경우 옵셔널 값을 반환
⇒ 이를 통해 호출하는 쪽에서 값이 없을 수 있음을 인지하고 적절한 방법으로 이를 처리할 수 있게 해야 함
• 널 값이나 비어 있는 옵셔널을 반환하는 것의 단점은 값이 없는 이유를 명시적으로 전달하지 않는다는 점이다. 사용자의 나이가 값을 제공하지 않았기 때문에 널인지, 혹은 시스템에서 오류가 발생하여 널인지 알 수 없다. 이러한 상황을 구별하는 것이 필요하다면 4장에서 설명한 오류전달 기법을 사용하는 것을 고려해야 함

때때로 매직값이 우연히 발생할 수 있다.

fun minValue(values: List<Int>): Int  {
	var minValue = Int.MAX_VALUE;
	for (value in values) {
		minValue = Math.min(value, minValue)
	}
	return minValue
}

• 리스트가 비어 있다면 Int.MAX_VALUE가 반환되게 된다.

• 매직값을 반환하는 것이 때로는 개발자가 의식적으로 내린 결정이지만, 어떤 경우에는 우연히 일어날 수 있다. 이유 여하를 막론하고 매직값은 예측을 벗어나는 결과를 초래할 수 있기 때문에 발생 가능한 상황에 대해 조심하는 것이 좋다.

2. 널 객체 패턴을 적절히 사용하라

빈 컬렉션을 반환하면 코드가 개선될 수 있다

Set<String>? getClassNames(HtmlElement element) {
	String? attribute = element.getAttribute("class");
	if (attribute == null) {
		return null;
	}
	return new Set(attribute.split(" "));
}

Boolean isElementHighlighted(HtmlElement element) {
	Set<String>? classNames = getClassNames(element);
	if (classNames == null) {
		return false;
	}
	return classNames.contains("highlighted");
}

• 널값 반환하면 이점이 있다고 주장할 수 있다
• class 속성이 지정되지 않은 경우 널값을 반환하고
• class 속성이 지정되어 있으나 비어있는 경우에 빈집 합 반환되도록
• 이 두 가지 경우의 미묘한 차이를 구분할 수 있다는 것이다. 하지만 그것이 전부다. 별 의미가 없다

Set<String> getClassNames(HtmlElement element) {
	String? attribute = element.getAttribute("class");
	if (attribute == null) {
		retrun new Set();
	}
	return new Set(attribute.split(" "));
}

Boolean isElementHighlighted(HtmlElement element) {
	return getClassNames(element).contains("highlighted")
}

빈 문자열을 반환하는 것도 때로는 문제가 될 수 있다

문자들의 모음으로서의 문자열

class UserFeedback {
	private String? additionalComments;
	
	String getAdditionalComments() {
		if (additionalComments == null) {
			return "";
		}
	return additionalComments;
	}
}

ID 로서의 문자열

class Payment {
	private final String? cardTransaciontId;
	
	String getCardTransactionId() {
		if (cardTransactionId == null) {
			return "";
		}
		return cardTransactionId;
	}
}

• 이 경우 cardTransactionId 문자열은 단순한 문자의 집합이 아니라 특정한 의미를 가지며 널을 갖는다는 것은 중요한 무언가를 의미함 ⇒ 함수가 널을 반환하는 것이 훨씬 더 낫다.

더 복잡한 널 객체는 예측을 벗어날 수 있다

class CoffeeMug {
	CoffeeMug(Double diameter, Double height) { ... }
	
	Double getDiameter() { ... }
	Double getHeight() { ... }
	
}

class CoffeeMugInventory {
	private final List<CoffeeMug> mugs;
	
	CoffeeMug getRandomMug() {
		if (mugs.isEmpty()) {
			return new CoffeeMug(diameter: 0.0, height: 0.0);
		}
		return mugs[Math.randomInt(0, mugs.size())];
	}
}

• 차라리 null을 반환해라. 이 함수는 호출할 때 언제나 유효한 CoffeeMug 를 반환받을 것이라는 잘못된 인상을 줄 수 있다.

널 객체 구현은 예상을 벗어나는 동작을 유발할 수 있다

interface CoffeeMug {
	Double getDiameter();
	Double getHeight();
	void reportMugBroken();
}

class CoffeeMugImpl implements CoffeeMug {
	override Double getDiameter() { return diameter; }
	override Double getHeight() { return height; }
	override void reportMugBroken() { ... }
}

class NullCoffeeMug implement CoffeeMug {
	override Double getDiameter() { return 0.0; }
	override Double getHeight() { return 0.0; }
	override void reportMugBroken() { 
		// do nothing
	}
}

• 개선된 점 한가지는 반환값이 NullCoffeeMug인지 확인함으로써 널 객체를 가졌는지 확인할 수 있다는 점이다. 그러나 호출하는 쪽에서 이 내용을 확인하고 싶어할 지가 전혀 명확지 않기 때문에 그다지 큰 개선 사항은 아니다.

• 널 객체 패턴은 여러 형태로 나타날 수 있다. 널 안정성과 옵셔널을 사용하는 것이 인기를 얻음에 따라 ‘값이 없음’을 훨씬 쉽고 안전하게 나타낼 수 있게 되었다. 이와 함께 널 객체 패턴의 사용을 지지하는 기존의 주장들 중 많은 것들이 요즘에는 설득력이 떨어졌다.

3. 예상치 못한 SideEffect를 피하라

• 사용자에게 출력 표시

• 파일이나 데이터베이스에 무언가를 저장

• 다른 시스템을 호출하여 네트워크 트래픽 발생

• 캐시 업데이트 혹은 무효화

분명하고 의도적인 부수 효과는 괜찮다

class UserDisplay {
	private final Canvas canvas;
	
	void displayErrorMessage(String message) {
		canvas.drawText(message, Color.RED); // 부수 효과: 캔버스가 업데이트된다.
	}
}

• 위 함수는 분명하고 의도적으로 부수 효과를 일으키는 예이다. 오류 메시지로 캔버스를 업데이트하는 것은 호출하는 쪽에서 원하고 예상하는 바이다.

예기치 않은 부수 효과는 문제가 될 수 있다

class UserDisplay {
	private final Canvas canvas;
	
	Color getPixel(Int x, Int y) {
		canvas.redraw();
		PixelData data = canvas.getPixel(x, y);
		return new Color(data.getRed(), data.getGreen(), data.getBlue());
	}
}

• 위와 같이 예상치 못한 부수 효과가 문제의 소지가 될 수 있는 몇가지 경우가 있다.

부수 효과는 비용이 많이 들 수 있다

class UserDisplay {
	private final Canvas canvas;
	
	Color getPixel(Int x, Int y) { ...	}
	
	Image captureScreenshot() {
		Image image = new Image(canvas.getWidth(), canvas.getHeight());
		for (Int x= 0; x < image.getWidth(); ++x) {
			for (Int y= 0; y < image.getHeight(); ++y) {
				image.setPixel(x,y, getPixel(x,y)); // getpixel 이 여러번 호출
			}
		}
	}
}

• getPixel 이 10ms 가 소요된다고 가정할때 captureScreenShot을 호출하면(400 x 700 pixel에 대하여) 47분간 애플리케이션이 멈추고 깜빡거리게 됨(redraw를 계속 호출..)

호출한 쪽의 가정을 깨뜨리기

 class UserDisplay {
	private final Canvas canvas;
	
	Color getPixel(Int x, Int y) { ...	}
	
	Image captureScreenshot() {
		Image image = new Image(canvas.getWidth(), canvas.getHeight());
		for (Int x= 0; x < image.getWidth(); ++x) {
			for (Int y= 0; y < image.getHeight(); ++y) {
				image.setPixel(x,y, getPixel(x,y)); // getpixel 이 여러번 호출
			}
		}
	}
	
	List<Box> getPrivacySensitiveAreas() { ... }
	
	Image captureRedactedScreenshot() {
		for (Box area in getPrivacySensitiveAreas()) {
			canvas.delete()
		}
		Image screenshot = captureScreenshot();
		canvas.redraw();
		return screenshot;
	}
}

• 위 코드에서 redraw에서 픽셀이 다시 보이는 것으로 예상하지만, captureScreenshot에서 이미 redraw가 수행되기에 픽셀이 보이게되고 screenshot에 개인정보가 포함되는문제가 발생한다
• 심각한 사용자 개인정보 침해이며 중대한 버그이다.

다중 스레드 코드의 버그

• 함수에 대한 개별 호출 문제에서 멀티스레딩 문제가 발생할 확률은 일반적으로 매우 낮다. 그러나 함수를 수천 번 호출할 경우 발생확률이 누적되어 상당히 높아진다. 다중 스레드 문제와 관련된 버그는 디버깅과 테스트가 어렵기로 악명 높다.

• captureScreenshot을 두 스레드에서 동시에 호출하면 다른 쪽에서 redraw할때, 한 스레드에서 다른 redraw하고 나서의 값을 읽어버릴 수 있기 때문에 원래 읽으려고 했던 값과 다른 값을 읽을 수가 있는 것

해결책 : 부수 효과를 피하거나 그 사실을 분명하게 하라

4. 입력 매개변수를 수정하는 것에 주의하라

입력 매개변수를 수정하면 버그를 초래할 수 있다

List<Invoice> getBillableInvoices(
	Map<User, Invoice> userInvoices,
	Set<User> usersWithFreeTrial) {
	userInvoices.removeAll(usersWithFreeTrial); // UserInvoice 를 변경
	return userInvoices.values();
}

void processOrders(OrderBatch orderBatch) {
	Map<User, Invoice> userinvoices = 
		orderBatch.getUserInvoices();
	Set<User> usersWithFreeTrial =
		orderBatch.getFreeTrialUsers();
	
	sendInvoices(getBillableInvoices(userInvoices,usersWithFreeTrial));
	enableOrderedServices(userInvoices)
}

해결책: 변경하기 전에 복사하라

List<Invoice> getBillableInvoices(
	Map<User, Invoice> userInvoices,
	Set<User> usersWithFreeTrial) {
	return userInvoices
			.entries()
			.filter(entry ->
				!usersWithFreeTrial.contains(entry.getKey()))
			.map(entry -> entry.getValue());
}

• 값 복사하면 메모리나 cpu 혹은 두 가지 모두와 관련해 성능에 악영향을 미칠 수 잇다. 하지만 입력 매개변수의 변경으로 인해 발생할 수 잇는 예기치 못한 동작이나 버그와 비교하면 성능은 크게 문제 x

• 만약 성능상의 이유로 입력 매개변수를 변경해야 하는 경우 함수 이름과 문서에 이러한 일이 발생한다는 점을 분명히 하는 것이 좋음

5. 오해를 일으키는 함수는 작성하지 말라

중요한 입력이 누락되었을 때 아무것도 하지 않으면 놀랄 수 있다

class UserDisplay {
	private final LocalizedMessage messages;
	
	void displayLegalDisclaimer(String? legalText) {
		if (legalText == null) { // null 일 때 아무것도 하지 않는다..
			return 
		}
		displayOverlay(
			title: messages.getLegalDisclaimerTitle(),
			message: legalText,
			textColor: Color.RED);
		}
	}
}

class LocalizedMessages {
	String getLegalDisclaimerTitle();
}

class SignupFlow {
	private final UserDisplay userDisplay;
	private final LocalizedMessages messages;
	
	void ensureLegalCompliance() {
		userDisplay.displayLegalDisclaimer(
			messages.getSignupDisclaimer()); // 코드는 고지 사항을 항상 보여주는 것처럼 보이지만
			// 실제로는 그렇지 않음
	}
}

해결책 : 중요한 입력은 필수 항목으로 만들라

• 중요한 매개변수가 널이 가능한 값을 받아들일 수 있게 하면 호출하는 쪽에서는 호출하기 전에 널값 여부를 확인할 필요가 없다. 이렇게 하면 호출하는 쪽의 코드는 간단해지는 반면 오해를 초래할 수 있음

class SignupFlow {
	private final UserDisplay userDisplay;
	private final LocalizedMessages messages;
	
	void ensureLegalCompliance() {
		String? signupDisclaimer = messages.getSignupDislcaimer();
		if (signupisclaimer == null) { 
			return false;
		}
		
		userDisplay.displayLegalDisclaimer(signupDisclaimer);
		return true;
	}
}

미래를 대비한 열거형 처리

미래에 추가될 수 있는 열거값을 암묵적으로 처리하는 것은 문제가 될 수 있다

enum PredictedOutcome {
	COMPANY_WILL_GO_BUST,
	COMPANY_WILL_MAKE_A_PROFIT
}

Boolean isOutcomeSafe(PredictedOutcome prediction) {
	if (prediction== PredictedOutcome.COMPANY_WILL_GO_BUST) {
		return false;
	}
	return true;
}

• 위 코드는 열거값이 2개인 동안에는 잘 동작하지만 그러나 만약 누군가가 열거값을 새로 추가한다면 상황은 심각하게 잘못될 수 있다.

enum PredictedOutcome {
	COMPANY_WILL_GO_BUST,
	COMPANY_WILL_MAKE_A_PROFIT
	WORLD_WILL_END
}

• 위와 같이 열거값이 추가되었는데 isOutcomeSafe가 수정되지 않으면 문제가 발생할 수 있다.

• 두 코드가 동시에 수정된다는 보장을 할 수 없다

해결책 : 모든 경우를 처리하는 스위치 문을 사용하라

• 위 코드의 문제점은 isOutcomeSafe() 함수가 열거형의 일부 값을 명시적이 아닌 암시적인 방식으로 처리한다는 점이다.

Boolean isOutcomeSafe(PredictedOutcome prediction) {
	switch (prediction) {
		case COMPANY_WILL_GO_BUST:
			return false;
		case COMPANY_WILL_MAKE_A_PROFIT:
			return true;
	}
	throw new UncheckedException("Unhandled prediction: " + predicition);
}

• 이 코드는 각각의 열거값을 사용하여 함수 호출을 수행하는 단위 테스트와 결합할 수 있다.

testIsOutcomeSafe_allPredictedOutcomeValues() {
	for (PredictedOutcome prediction in PredictedOutcome.values()) {
		isOutcomeSafe(prediction);
	}
}

기본 케이스를 주의하라

Boolean isOutcomeSafe(PredictedOutcome prediction) {
	switch (prediction) {
		case COMAPNY_WILL_GO_BUST:
			return false;
		case COMPANY_WILL_MAKE_A_PROFIT:
			return true;
		default:
			return false; // 새로운 열거형 값에 대해 기본적으로 거짓을 반환한다.
	}
}

• 기본 케이스를 사용하면 새로운 값에 대해서 암시적으로 처리하기 때문에 예상을 벗어나는 동작과 버그를 초래할 수 있다

기본 (default) 케이스에서 예외 발생

Boolean isOutcomeSafe(PredictedOutcome prediction) {
	switch (prediction) {
		case COMAPNY_WILL_GO_BUST:
			return false;
		case COMPANY_WILL_MAKE_A_PROFIT:
			return true;
		default:
			throw new UncheckedException("...")
	}
}

• 이 방식이 위에서 switch 문 이후에 Exception을 던지는 코드와 차이가 없어보이지만, 위와 같이 코드를 작성하면 컴파일러의 추가적인 보호를 받을 수없다
• 컴파일러는 나중에 열거형이 추가되었을 때도 스위치 문이 모든 케이스를 다 처리한다고 판단을 하기 때문에 경고를 출력하지 않게 된다.
• 컴파일러가 처리되지 않은 열거값에 대해 경고를 여전히 출력하도록 하려면 스위치 문이 끝나고 Exception을 던지는 편이 더 낫다.

이 모든 것을 테스트로 해결할 수는 없는가?

• 예상을 벗어나는 코드를 방지하기 위한 코드품질 향상 노력에 반대하는 주장을 하는 사람들이 가끔 있다. 테스트가 이러한 모든 것을 잡아낼 것이기 때문에 이런 노력은 시간 낭비라는 것이다 (그럼 테스트는 잘짜려나..?)

• 직관적이지 않거나 예상을 벗어나는 코드에 숨어 있는 오류를 테스트만으로는 방지하기 어렵다.