Hello, I'm Munetoshi Ishikawa, a mobile client developer for the LINE messaging app.
This article is the latest installment of our weekly series "Improving code quality". For more information about the Weekly Report, please see the first article.
Be mindful of keeping a proper distance
Let's assume you need to implement a UI that displays a sorted list of Item
s. At the top of this Item
list, there is a header showing the total number of Item
s. Here is an example of the list display:
Items: 3
---------
<item1>
---------
<item2>
---------
<item3>
Additionally, the list and total count display have the following specifications:
- The number of
Item
s that can be displayed in the list is up to the first 100 items. - If there are more than 100
Item
s, the count display will be "100+".
The display format when the total count exceeds 100 is as follows:
Items: 100+
---------
<item1>
---------
<item2>
---------
... (omitted) ...
---------
<item100>
To achieve this specification, we defined the model classes Item
and StoredItems
and the Repository
as follows:
// 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)
}
}
On the other hand, the logic for determining the total count text on the UI side branches based on whether the number of elements exceeds 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)
Is there any problem with this code?
Keep a distance of +1
This code shares "implicit knowledge" between the UI and repository layers, making it prone to bugs when specifications change. When dividing code into multiple layers or components, consider "which layer/component should have what knowledge".
The repository layer should not be aware of UI details, but the comment // `+ 1` is for showing "+" on the UI.
in ItemRepository
indicates otherwise. Conversely, the UI side also depends on the details of the repository layer. The logic for determining countTextView.text
on the UI side depends on the behavior of the repository layer, which returns a list larger than ITEM_LIST_MAX_COUNT
if there are more than ITEM_LIST_MAX_COUNT
Item
s.
To solve this problem, you can add a property to StoredItems
that indicates whether there are more Item
s than can fit in the list.
class StoredItems(val items: List<Item>, val hasMoreItems: Boolean)
By doing this, the repository layer can focus on creating instances of the model class without being aware of UI details.
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
)
}
}
Furthermore, the UI layer no longer needs to have knowledge of ITEM_LIST_MAX_COUNT
.
val countText = storedItems.items.size.toString()
countTextView.text = if (storedItems.hasMoreItems) "$countText+" else countText
itemListAdapter.items = storedItems.items
But is it okay for ITEM_LIST_MAX_COUNT to be in the repository layer?
There are several options for where to hold the knowledge of ITEM_LIST_MAX_COUNT
. For example, you could define ITEM_LIST_MAX_COUNT
in a business logic layer. The business logic layer can take various forms depending on the architecture adopted, such as domain, service, or use case.
If setting up a business logic layer is overkill, another option is to have the model class hold this knowledge.
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
}
}
However, this method requires caution as it can blur the direction of dependency from "algorithm -> data structure". In particular, be careful not to include feature-specific logic in a data model that is used generically.
Summary
Avoid code that implicitly depends on the detailed behavior of another layer.
Keywords: implicit dependency
, module structure
, responsibility