The original article was published on April 3, 2025.
Hello, I'm Yūdai Takanashi, 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.
Collection is not just about list
We are trying to implement a function called bulkQuery() to retrieve user data in bulk. However, each user belongs to categories such as FRIEND or RECOMMENDED, and we want to be able to retrieve user data by category. In the implementation of bulkQuery() below, it takes a list of categories as an argument and returns a list of sets of users. The index of the argument list corresponds to the index of the return list.
fun bulkQuery(categories: List<UserCategory>): List<Set<User>> = TODO()
enum class UserCategory { FRIEND, RECOMMENDED, BLOCKED }
data class User(val id: Int)The following code is an example of using bulkQuery(). Here, it retrieves users from the FRIEND and RECOMMENDED categories and displays them.
// Caller
val targetCategories = listOf(UserCategory.FRIEND, UserCategory.RECOMMENDED)
val result = bulkQuery(targetCategories)
targetCategories.zip(result).forEach { (category, users) ->
val sortedUsers = users.sortedBy(User::id)
when (category) {
UserCategory.FRIEND -> println("Friends: $sortedUsers")
UserCategory.RECOMMENDED -> println("Recommended: $sortedUsers")
UserCategory.BLOCKED -> error("error")
}
}Is there room for improvement in this code?
Utilizing the characteristics of collections
Collections have types such as List and Set, each with different characteristics. However, the current bulkQuery() does not effectively utilize these characteristics. Depending on the specifications, you can make better use of the characteristics of collections by changing the argument and return types as follows:
- fun bulkQuery(categories: List<UserCategory>): List<Set<User>>
+ fun bulkQuery(categories: Set<UserCategory>): Map<UserCategory, List<User>>Change of argument (List<UserCategory> -> Set<UserCategory>)
Passing a list containing the same UserCategory as an argument is meaningless. Using a Set emphasizes that duplicates do not matter.
Change of return value (List<Set<User>> -> Map<UserCategory, List<User>>)
In the original implementation, user data within each category was returned as a Set. The caller code uses this Set after sorting by ID. When order matters, it is better to use a List. Especially when using a database in the data layer, sorting there is often more performant. Adding a comment explaining the order of the sorted list makes the code more readable.
Another improvement is changing the grouping by category from a List to a Map, eliminating the implicit relationship between arguments and return values. In the List implementation, to know which user belongs to which category, you have to check the correspondence with the argument index. The caller code uses zip() to match the query results, but this structure makes it difficult to reuse the results later. Such implicit relationships are not only hard to understand but also fragile, requiring detailed comments to explain the relationship. Using a Map to explicitly show the relationship between the query and the results makes the code easier to understand and less prone to breakage.
In a nutshell
When using collections, be aware of the differences in characteristics and choose the appropriate type.
Keywords: collection, implicit relationship