안녕하세요. 커뮤니케이션 앱 LINE의 모바 일 클라이언트를 개발하고 있는 Ishikawa입니다.
저희 회사는 높은 개발 생산성을 유지하기 위해 코드 품질 및 개발 문화 개선에 힘쓰고 있습니다. 이를 위해 다양한 노력을 하고 있는데요. 그중 하나가 Review Committee 활동입니다.
Review Committee에서는 머지된 코드를 다시 리뷰해 리뷰어와 작성자에게 피드백을 주고, 리뷰하면서 얻은 지식과 인사이트를 Weekly Report라는 이름으로 매주 공유하고 있습니다. 이 Weekly Report 중 일반적으로 널리 적용할 수 있는 주제를 골라 블로그에 코드 품질 개선 기법 시리즈를 연재하고 있습니다.
이번에 블로그로 공유할 Weekly Report의 제목은 '적절한 거리 유지에 신경 쓰자'입니다.
적절한 거리 유지에 신경 쓰자
정렬된 Item 목록을 표시하는 UI를 구현한다고 가정해 봅시다. 이 Item 목록 상단에는 Item의 총 개수를 표시하는 헤더가 있습니다. 다음은 목록 표시 예시입니다.
Items: 3
---------
<item1>
---------
<item2>
---------
<item3>목록 및 총 개수 표시에는 다음과 같은 사양이 있습니다.
- 목록에 표시할 수 있는
Item개수는 처음 100개까지 - 100 개를 초과하는
Item이 있는 경우 총 개수는 "100+"로 표시
총 개수가 100을 초과할 때 표시 형식은 다음과 같습니다.
Items: 100+
---------
<item1>
---------
<item2>
---------
... (생략) ...
---------
<item100>이 사양을 구현하기 위해 모델 클래 스 Item과 StoredItems 및 Repository를 다음과 같이 정의했습니다.
// Model classes
const val ITEM_LIST_MAX_COUNT = 100
class Item(...)
class StoredItems(val items: List<Item>)// In the repository layer
class ItemRepository(...) {
suspend fun getItemList(): StoredItems {
// `+ 1` is for showing "+" on the UI.
val items = itemDao.selectItems(ITEM_LIST_MAX_COUNT + 1)
return StoredItems(items)
}
}한편 UI에서 총 개수를 표시하는 텍스트를 결정하는 로직은 Item 개수가 ITEM_LIST_MAX_COUNT를 초과하는지 아닌지에 따라 분기됩니다.
val itemCount = storedItems.items.size
countTextView.text =
if (itemCount <= ITEM_LIST_MAX_COUNT) itemCount.toString() else "$ITEM_LIST_MAX_COUNT+"
itemListAdapter.items = storedItems.items.take(ITEM_LIST_MAX_COUNT)이 코드에 문제가 있을까요?
'+1'만큼의 거리를 유지하자
코드를 여러 레이어나 컴포넌트로 나눌 때에는 '어떤 레이어나 컴포넌트가 어떤 정보를 가져야 하는지'를 고려해서 설계하는 것이 좋습니다. 하지만 위 코드는 UI와 리포지터리 레이어 간에 '암묵적인 정보'를 공유하고 있기 때문에 사양을 변경했을 때 버그가 발생하기 쉬운 구조입니다.
구체적으로 살펴보겠습니다. 먼저 리포지터리 레이어는 UI의 세부 사항에 대해 알 필요가 없는데요. // `+ 1` is for showing "+" on the UI.라는 주석은 이에 반해 세부 사항을 알리고 있습니다.
반대로 UI 측도 리포지터리 레 이어의 세부 사항에 의존하고 있습니다. UI 측에서 countTextView.text를 구하는 로직이 리포지터리 레이어의 'ITEM_LIST_MAX_COUNT를 초과하는 Item이 있을 경우 ITEM_LIST_MAX_COUNT보다 큰 목록을 반환한다'는 동작에 의존하고 있기 때문입니다.
이 문제를 해결하기 위해서는 다음과 같이 '목록에 다 들어갈 수 없는 Item이 있는지'를 나타내는 속성을 StoredItems에 추가하면 됩니다.
class StoredItems(val items: List<Item>, val hasMoreItems: Boolean)이렇게 하면 리포지터리 레이어는 UI 세부 사항을 신경 쓰지 않고 모델 클래스의 인스턴스 생성에 집중할 수 있습니다.
private const val ITEM_LIST_MAX_COUNT = 100
class ItemRepository(...) {
suspend fun getItemList(): StoredItems {
// `+ 1` is for deciding `hasMoreItems`.
val items = itemDao.selectItems(ITEM_LIST_MAX_COUNT + 1)
return StoredItems(
items.take(ITEM_LIST_MAX_COUNT),
items.size > ITEM_LIST_MAX_COUNT
)
}
}또한 UI 레이어는 더 이상 ITEM_LIST_MAX_COUNT에 대한 정보가 없어도 됩니다.
val countText = storedItems.items.size.toString()
countTextView.text = if (storedItems.hasMoreItems) "$countText+" else countText
itemListAdapter.items = storedItems.items그런데 ITEM_LIST_MAX_COUNT는 리포지터리 레이어에 있어도 괜찮을까?
ITEM_LIST_MAX_COUNT 관련 정보를 어디에 넣을지에 대해서는 몇 가지 선택지가 있습니다.
예를 들어 비즈니스 로직 레이어를 마련하고 그곳에서 ITEM_LIST_MAX_COUNT를 정의할 수 있습니다. 비즈니스 로직 레이어는 채택하는 아키텍처에 따라 도메인, 서비스, 유스 케이스 등 다양한 형태가 될 수 있습니다.
만약 비즈니스 로직 레이어를 만드는 것이 과하다고 생각한다면 모델 클래스에 해당 정보를 넣는 것도 한 가지 방법입니다.
class StoredItems(storedItemList: List<Item>) {
val items: List<Item> = storedItemList.take(MAX_ITEM_COUNT)
val hasMoreItems: Boolean = storedItemList.size > MAX_ITEM_COUNT
companion object {
private const val MAX_ITEM_COUNT = 100
const val ITEM_COUNT_FOR_QUERY = MAX_ITEM_COUNT + 1
}
}다만 이 방법은 '알고리즘 -> 데이터 구조'라는 의존 관계의 방향이 모호해지기 때문에 주의해야 합니다. 특히 기능 고유의 로직을 범용적으로 사용되는 데이터 모델에 포함시키지 않도록 조심해야 합니다.
한 줄 요약: 다른 레이어의 세부 동작에 암묵적으로 의존하는 코드는 피해야 한다.
키워드:
implicit dependency,module structure,responsibility
