이 글은 2024년 2월 8일에 일본어로 먼저 발행된 기사를 번역한 글입니다.
안녕하세요. 커뮤니케이션 앱 LINE의 모바일 클라이언트를 개발하고 있는 Ishikawa입니다.
저희 회사는 높은 개발 생산성을 유지하기 위해 코드 품질 및 개발 문화 개선에 힘쓰고 있습니다. 이를 위해 다양한 노력을 하고 있는데요. 그중 하나가 Review Committee 활동입니다.
Review Committee에서는 머지된 코드를 다시 리뷰해 리뷰어와 작성자에게 피드백을 주고, 리뷰하면서 얻은 지식과 인사이트를 Weekly Report라는 이름으로 매주 공유하고 있습니다. 이 Weekly Report 중 일반적으로 널리 적용할 수 있는 주제를 골라 블로그에 코드 품질 개선 기법 시리즈를 연재하고 있습니다.
이번에 블로그로 공유할 Weekly Report의 제목은 '세트 할인'입니다.
세트 할인
디버깅용 데이터를 시간 순서대로 기록하는 클래스인 SampledDataRecorder를 만든다고 가정해 봅시다. 이 클래스는 recordData로 전달된 데이터를 기록하는데 이때 DataImportance에 따라 필터링하고 dataCountPerSampling 속성을 이용해 지정된 간격으로 샘플링합니다.
enum class DataImportance(val level: Int) { TRIVIAL(0), NORMAL(1), IMPORTANT(2) }
class SampledDataRecorder<T>(private val record: DataRecord<T>) {
var isActive: Boolean = false
var minImportanceToRecord: DataImportance = DataImportance.NORMAL
@IntRange(from = 1)
var dataCountPerSampling: Int = 1
set(value) {
currentDataCount = 0
field = value.coerceAtLeast(1)
}
private var currentDataCount: Int = 0
fun recordData(importance: DataImportance, data: T) {
if (!isActive || importance.level < minImportanceToRecord.level) {
return
}
if (currentDataCount + 1 >= dataCountPerSampling) {
currentDataCount = 0
record.store(data)
} else {
currentDataCount++
}
}
}recordData에 전달된 데이터는 다음 세 가지 조건이 충족될 때 기록됩니다.
isActive가true- 인수의
importance가minImportanceToRecord이상 recordData의 호출이dataCountPerSampling* n번째(n은 양의 정수)
여기서 isActive, minImportanceToRecord, dataCountPerSampling 의 세 가지 속성은 디버깅 UI에서 변경할 수 있도록 재할당이 가능하다는 필수 제약 조건이 있다고 가정합니다.
이 SampledDataRecorder에 개선해야 할 점이 있을까요?
개별 변경은 비용이 많이 든다
isActive, minImportanceToRecord, dataCountPerSampling, 이 세 가지는 독립적으로 변경할 수 있기 때문에 혼란을 초래할 수 있습니다. 이런 속성은 하나의 클래스로 묶어 한 번에 업데이트하도록 하면 동작을 단순화할 수 있습니다.
위 코드에서 혼동하기 쉬운 부분은 다음과 같습니다.
isActive를false에서true로 변경할 때 이전minImportanceToRecord및dataCountPerSampling값이 사용될 수 있다.minImportanceToRecord나dataCountPerSampling을 변경하고isActive를true로 설정해야 하는데 실수로isActive를 먼저 변경하면 이전minImportanceToRecord및dataCountPerSampling값을 사용하게 된다.
currentDataCount는dataCountPerSampling이 변경될 때에는 재설정되지만,isActive또는minImportanceToRecord가 변경될 때에는 재설정되지 않는다.- 속성 설정자(property setter)가 비 동기 방식으로 호출될 경우 단일 스레드 환경일지라도 경쟁 상태에 주의해야 한다.
위 문제를 해결하려면 minImportanceToRecord와 dataCountPerSampling을 하나의 클래스인 SamplingPolicy로 묶고 isActive는 null로 표현하면 됩니다.
class SampledDataRecorder<T>(private val record: DataRecord<T>) {
/**
* Current policy of ...(snip)...
* `null` value represents this recorder is inactive.
*/
private var policy: SamplingPolicy? = null
private var currentDataCount: Int = 0
fun startRecording(dataCountPerSampling: Int, minImportanceToRecord: DataImportance) {
policy = SamplingPolicy(dataCountPerSampling.coerceAtLeast(1), minImportanceToRecord)
currentDataCount = 0
}
fun finishRecording() {
policy = null
}
fun recordData(importance: DataImportance, data: T) {
val currentPolicy = policy ?: return
if (importance.level < currentPolicy.minImportanceToRecord.level) {
return
}
if (currentDataCount + 1 >= currentPolicy.dataCountPerSampling) {
currentDataCount = 0
record.store(data)
} else {
currentDataCount++
}
}
private class SamplingPolicy(
@IntRange(from = 1) val dataCountPerSampling: Int,
val minImportanceToRecord: DataImportance
)
}위 코드는 다음과 같은 장점이 있습니다.
dataCountPerSampling과minImportanceToRecord가 항상 함께 업데이트되도록 보장한다.- 기록을 재개할 때 의도치 않게 이전 정책이 사용되는 버그를 방지할 수 있다.
currentDataCount가 재설정되는 조건이 명확하다.- 비동기 업데이트 처리가 쉬워지고, 해당 클래스를 스레드 안전(thread-safe)하게 만들고 싶은 경우에 변경 부분을 제한할 수 있다.
정리
상태를 변경할 때 개별 속성을 직접 변경하게 만드는 것보다 변경을 제한하는 인터페이스를 만드는 게 더 좋은 경우가 있습니다. 각 속성이 서로 독립적이지 않아서 속성 중 하나의 값이 결정되면 그것이 다른 속성의 값에 영향을 미치거나(참고: Code readability: Session 4 의 슬라이드 15-32) 또는 여러 속성을 한 번에 변경해야 할 경우에 특히 유용합니다.
한 줄 요약: 상태 업데이트 시점이나 값의 조합을 제한하는 인터페이스를 제공하는 것이 좋은 경우가 있다.
키워드:
mutability,updating interface,state object