tech blog 글 읽고 정리하기
#
인자가 많은 메서드는 왜 나쁠까? #
상황 #
재전송, 메일 수신자 필터링, SMS 전송(fallback) 등 다양한 기능을 제공하는 메일 발송 기능이 있을때. 이 기능을 하는 메서드의 인자가 11개 정도 있다면?
class Mail(
// ...
) {
fun send(
phoneFallback: Boolean?,
phoneNumber: String?,
isForceSend: Boolean?,
recipient: String,
id: Long,
mailDomainFilterService: MailDomainFilterService?,
mailRetryService: MailRetryService?,
title: String,
body: String,
param: Map<Any, Any>,
reservedAt: Instant?,
)
}
- 위 클래스를 다음과 같이 호출한다.
mail.send(
phoneFallback = null,
phoneNumber = null,
isForceSend = null,
recipient = "jaeeun.na@tosspayments.com",
id = -1,
mailDomainFilterService = null,
mailRetryService = null,
title = "안녕하세요",
body = "메일 본문입니다",
param: emptyMap(),
reservedAt: null,
)
- 각각의 파라미터가 어떤 역할을 하는지 알겠는가?
- 메일 제목, 내용, 받는사람만 채워서 메일 한통을 보내고 싶을 뿐인데, 이렇게 인자가 많고 의미를 파악하기 어려우면 메서드 사용성이 떨어진다.
샌상성 악마의 미소 #
- 비슷한 역할을 하는 Gmail UI를 한번 보자.
- 받는 사람만 입력하면 본문 내용이 없어도 메일이 발송된다. 어떤 값이 필수인지도 바로 알 수 있다.
class Mail(
// ...
) {
fun send(
phoneFallback: Boolean?,
phoneNumber: String?,
isForceSend: Boolean?,
recipient: String,
id: Long,
mailDomainFilterService: MailDomainFilterService?,
mailRetryService: MailRetryService?,
title: String,
body: String,
param: Map<Any, Any>,
reservedAt: Instant?,
)
}
- Mail.send() 메서드에는 입력해야 하는 ‘필수’ 인자가 너무 많다. 잘 모르는인자, 사용할 필요 없는 인자에 null을 채워 넣는 것도 불편하다.
다른 선택지를 고민
- send 메서드는 다른 곳에서 어떻게 쓰고있을까? 비슷하게 따라해야겠다.
- 아무 인자나 넣고 일단 실행해볼까? 실행에 성공하면 좋고, 예외가 생긴다면 그때 확인해야지.
- send 메서드 구현은 어떻게 되어있지? 각 인자의 의미를 파악해보자.
어떤 선택지를 고르더라도 모두 생산성은 낮다. 어떤 선택지 하나를 고르는 순간, 생산성 악마는 이런 질문을 한다.
- send 메서드는 다른 곳에서 어떻게 쓰고있을까? 비슷하게 따라해야겠다.
- 메서드 실행에 과연 성공할까? 올바른지 검증할 수 있을까?
- 따라한 코드의 맥락과 사용하는 맥락이 같은가?
- 아무 인자나 넣고 일단 실행해볼까? 실행에 성공하면 좋고, 예외가 생긴다면 그때 확인해야지.
- 이 상태로 라이브 배포를 할 수 있을까?
- 내가 만드는 코드를 정확히 이해하지 못해도 괜찮은가?
- send 메서드 구현은 어떻게 되어있지? 각 인자의 의미를 파악해보자.
- 내부 구현을 정말로 이해할 수 있을까?
- 그 시간에 다른 기능을 만드는게 낫지 않을까?
질문에 답을 하다보면 세가지 선택지 모두 좋은 접근이 아니다. 문제를 해결하기 위해서는 근본적이고 장기적인 해결책이 필요하다.
함께 사용하는 인자는 하나로 묶는다. #
이해하기 어려운 인자부터 하나씩 봐보자.
- phoneFallback 인자가 뭘까?
- 메일 발송이 실패했을때 메일이 있는 내용을 문자로 대신 보내기 위한 인자
- phoneNumber 은?
- 위
phoneFallback
이 true라면phoneNumber
에 적힌 번호로 메일 내용이 전송된다.
즉, 1) phoneFallback
과 2) phoneNumber
은 항상 같이 사용되는 인자다.
개선한 코드
// 아래 두 변수가 둘 다 null이거나, 둘 다 null이 아니어야 한다는 사실을
// 더 이상 기억하고 있을 필요가 없다
data class FallbackFeatureOption(
val phoneFallback: Boolean,
val phoneNumber: String,
)
class Mail(
// ...
) {
fun send(
fallbackFeatureOption: FallbackFeatureOption?,
isForceSend: Boolean?,
recipient: String,
id: Long,
mailDomainFilterService: MailDomainFilterService?,
mailRetryService: MailRetryService?,
title: String,
body: String,
param: Map<Any, Any>,
reservedAt: Instant?,
) {
sendInternal(
phoneFallback = fallbackFeatureOption?.phoneFallback,
phoneNumber = fallbackFeatureOption?.phoneNumber,
isForceSend = isForceSend,
recipient = recipient,
id = id,
mailDomainFilterService = mailDomainFilterService,
mailRetryService = mailRetryService,
title = title,
body = body,
param = param,
reservedAt = reservedAt,
)
}
@Deprecated("이 메서드는 너무 인자가 많아서 코드를 이해하기 어렵습니다. 다른 send() 메서드를 사용하세요.")
fun sendInternal(
phoneFallback: Boolean?,
phoneNumber: String?,
isForceSend: Boolean?,
recipient: String,
id: Long,
mailDomainFilterService: MailDomainFilterService?,
mailRetryService: MailRetryService?,
title: String,
body: String,
param: Map<Any, Any>,
reservedAt: Instant?,
) { ... }
}
아직 완벽한 코드는 아니지만, 코드 사용자가 기억할 내용을 하나 줄였다.
fallbackFeatureOption에 null을 넣어야한다는게 맘에 안든다. 문자 발송은 더이상 신경쓰기 싫다.
fun send( // fallbackFeatureOption 인자를 삭제했다
// ...
) {
sendInternal(
phoneFallback = null,
phoneNumber = null,
// ...
)
}
// 메서드 이름으로 의도를 표현했다
fun sendWithFallback(
fallbackFeatureOption: FallbackFeatureOption, // non-nullable
// ...
) {
sendInternal(
phoneFallback = fallbackFeatureOption.phoneFallback,
phoneNumber = fallbackFeatureOption.phoneNumber,
// ...
)
}
send() 메서드가 노출하고 있던 phoneFallback 맥락이 사라졌다. -> sendWithFallback() 별도 메서드를 추가함 phoneFallback이 false 인데 phoneNumber 값을 넣어야해서 여전히 어색하다. 문자를 보내지 않을건데 휴대폰 번호를 넣어야하니깐. (조금 뒤에서 다시 개선해보자.)
관련 없는 것은 분리한다. #
isForceSend
은? 메일 수신을 거부한 이메일 주소에도 발송 기록을 남겨야하는 정책 때문에 사용하는 인자다. 즉,isForceSend
를 true로 설정하면 수신 거부한 주소에도 메일을 발송할 수 있다. 이메일 수신 거부 목록은 데이터베이스에 들어있고,MailDomainFilterService
에서 데이터베이스에 접근해서 발송할지 결정한다.
class MailDomainFilterService {
fun isFiltered(domain: String): Boolean {
// database 에서 목록을 가져온 뒤, 발송 여부 결정
}
}
isForceSend
가 true이면 mailDomainFilterService
인자가 사용되지 않는다.
-> isForceSend가 true여도 메일을 보내야하기 때문
class Mail(
private val mailDomainFilterService: MailDomainFilterService,
) {
fun send(
// ...
)
fun sendWithFallback(
// 기존과 동일
)
// 수신 거부 주소에도 메일 발송
fun sendWithFallbackAndForced(
fallbackFeatureOption: FallbackFeatureOption?,
recipient: String,
id: Long,
mailRetryService: MailRetryService?,
title: String,
body: String,
param: Map<Any, Any>,
reservedAt: Instant?,
) {
sendInternal(
phoneFallback = fallbackFeatureOption?.phoneFallback,
phoneNumber = fallbackFeatureOption?.phoneNumber,
isForceSend = true,
recipient = recipient,
id = id,
mailDomainFilterService = null,
mailRetryService = mailRetryService,
title = title,
body = body,
param = param,
reservedAt = reservedAt,
)
}
}
sendWithFallbackAndForced()
메서드를 추가해서, isForceSend
가 true
다.
문제 발생 #
fallbackFeatureOption
기능과 isForceSend
기능이 서로 겹치기 때문에,
다음과 같이 5개의 메서드를 만들어야하는 상황이다.
1) send()
2) sendInternal()
3) sendWithFallbackAndForced()
4) sendWithFallback()
5) sendWithForce()
연관이 없는 메서드들은 독립적으로 사용하도록 해보자.
class Mail(
private val mailDomainFilterService: MailDomainFilterService
) {
private var enableForceSendFeatureForCompliancePurpose: Boolean = false
private var smsFallbackFeatureOption: SmsFallbackFeatureOption? = null
fun enableSmsFallbackFeature(smsFallbackFeatureOption: SmsFallbackFeatureOption): Mail {
this.smsFallbackFeatureOption = smsFallbackFeatureOption
return this
}
// isForceSend 라는 이름에서, 조금 더 서술적인 이름으로 변경했다.
fun enableForceSendFeatureForCompliancePurpose(): Mail {
this.enableForceSendFeatureForCompliancePurpose = true
return this
}
fun send(
id: Long,
recipient: String,
mailRetryService: MailRetryService?,
title: String,
body: String,
param: Map<Any, Any>,
reservedAt: Instant?,
) {
// 이제 인자가 아닌 객체 필드에서 값을 얻어올 수 있다.
sendInternal(
phoneFallback = this.smsFallbackFeatureOption?.phoneFallback,
phoneNumber = this.smsFallbackFeatureOption?.phoneNumber,
isForceSend = this.enableForceSendFeatureForCompliancePurpose,
recipient = recipient,
id = id,
mailDomainFilterService = if (this.enableForceSendFeatureForCompliancePurpose) {
this.mailDomainFilterService
} else {
null
},
mailRetryService = mailRetryService,
title = title,
body = body,
param = param,
reservedAt = reservedAt,
)
}
}
SmsFallbackFeatureOption 클래스를 마저 개선해보자. #
// sms fallback 기능을 사용하겠다고 했는데 true를 입력하는게 어색하다.
mail
.enableSmsFallbackFeature(
SmsFallbackFeatureOption(true, "010-1234-5678")
)
.send(...)
// sms fallback 기능을 사용하겠다고 했는데 false를 입력하면 어떻게 되는거지...?
mail
.enableSmsFallbackFeature(
SmsFallbackFeatureOption(false, "???")
)
.send(...)
불필요한 인자를 삭제하고 이름을 개선하자. #
class Mail(
private val mailDomainFilterService: MailDomainFilterService
) {
private var enableForceSendFeatureForCompliancePurpose: Boolean = false
private var smsFallbackFeaturePhoneNumber: String? = null
fun enableSmsFallbackFeature(smsFallbackFeaturePhoneNumber: String): Mail {
this.smsFallbackFeaturePhoneNumber = smsFallbackFeaturePhoneNumber
}
fun enableForceSendFeatureForCompliancePurpose(): Mail {
this.enableForceSendFeatureForCompliancePurpose = true
}
fun send(
id: Long,
recipient: String,
mailRetryService: MailRetryService?,
title: String,
body: String,
param: Map<Any, Any>,
reservedAt: Instant?,
) {
sendInternal(
phoneFallback = this.smsFallbackFeaturePhoneNumber != null,
phoneNumber = this.smsFallbackFeaturePhoneNumber,
isForceSend = this.enableForceSendFeatureForCompliancePurpose,
recipient = recipient,
id = id,
mailDomainFilterService = if (this.enableForceSendFeatureForCompliancePurpose) {
this.mailDomainFilterService
} else {
null
},
mailRetryService = mailRetryService,
title = title,
body = body,
param = param,
reservedAt = reservedAt,
)
}
}
이제 좀더 편리하게 사용할 수 있게 되었다. 메서드 이름을 보고 충분히 기능을 유추할 수 있게 되엇고, send() 메서드의 주의사항을 더 기억하지 않아도 된다. 사용하고 싶은 기능을 호출하기만 하면 된다.
mail.enableSmsFallbackFeature("010-1234-5678") // 이 메서드를 호출하지 않아도 괜찮다
mail.enableForceSendFeatureForCompliancePurpose() // 이 메서드를 호출하지 않아도 괜찮다
.send(...)
가장 중요한 인자만 남긴다. #
class Mail(
private val mailDomainFilterService: MailDomainFilterService,
private val mailRetryService: MailRetryService,
private val param: Map<Any, Any>?,
) {
private var enableForceSendFeatureForCompliancePurpose: Boolean = false
private var smsFallbackFeaturePhoneNumber: String? = null
private var scheduleSendReservedAt: Instant? = null
private var enableRetryFeature: Boolean = false
fun enableSmsFallbackFeature(smsFallbackFeaturePhoneNumber: String): Mail {
this.smsFallbackFeaturePhoneNumber = smsFallbackFeaturePhoneNumber
}
fun enableForceSendFeatureForCompliancePurpose(): Mail {
this.enableForceSendFeatureForCompliancePurpose = true
}
fun enableScheduleSendFeature(reservedAt: Instant): Mail {
this.scheduleSendReservedAt = reservedAt
}
fun enableRetryFeature(): Mail {
this.enableRetryFeature = true
}
fun send(
recipient: String,
title: String,
body: String,
) {
sendInternal(
phoneFallback = this.smsFallbackFeaturePhoneNumber != null,
phoneNumber = this.smsFallbackFeaturePhoneNumber,
isForceSend = enableForceSendFeatureForCompliancePurpose,
id = Random.nextLong(), // 내부 구현에서 랜덤으로 생성해서 사용함
mailDomainFilterService = if (this.enableForceSendFeatureForCompliancePurpose) {
this.mailDomainFilterService
} else {
null
},
mailRetryService = if (this.enableRetryFeature) {
this.mailRetryService
} else {
null
},
title = title,
body = body,
param = emptyMap(),
reservedAt = this.scheduleSendReservedAt,
)
}
fun templateSend(
recipient: String,
title: String,
body: String,
param: Map<Any, Any>
) {
sendInternal(
// ...
param = param
// ...
)
}
}
sendInternal
메서드 구현을 전혀 건드리지 않고도 이젠 코드에 내가 원하는 값만 넣을 수 있게 되었다.
코드를 읽기도, 사용하기도 편해졌다.
// 가장 간단하게 사용할 때
mail.send(
recipient = "jaeeun.na@tosspayments.com",
title = "안녕하세요",
body = "메일 본문입니다"
)
// 지원하는 모든 기능을 사용할 때
mail.enableSmsFallbackFeature("010-1234-5678")
.enableForceSendFeatureForCompliancePurpose()
.enableScheduleSendFeature(Instant.now().plus(Duration.ofHours(2)))
.enableRetryFeature()
.send(
recipient = "jaeeun.na@tosspayments.com",
title = "안녕하세요",
body = "메일 본문입니다"
)