안녕하세요. 커뮤니케이션 앱 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)가 비