こんにちは。コミュニケーションアプリ「LINE」のモバイルクライアントを開発している石川です。
この記事は、毎週木曜の定期連載 “Weekly Report” 共有の第 18 回です。Weekly Report については、第 1 回の記事を参照してください。
関数を見て関係を見ず
コードを書く上で 、しばしばループのネストが必要になることがあります。「ページ」や「チャンク」で分割されたデータを処理することは、その典型例の 1 つに数えられます。
複数の Item
によってチャンクが作られ、更にそのチャンクが集まってページを構成するという状況を考えます。ページのデータモデル Page
は、以下のように定義されています。各 Page
は、index
によって順序付けられ、後続の Page
が存在するならば (つまり、この Page
が最後でないならば)、hasNext
が true
になります。
class Page(val index: UInt, val chunks: List<Item>, val hasNext: Boolean)
この Page
を使って、すべての Item
のメタデータを保存する関数 saveAllItemMetadata
が以下のように実装されました。この関数では、page.hasNext
が true
であり続ける限り、次の Page
を取得し、そこに含まれる Item
のメタデータを保存しています。
fun saveAllItemMetadata() {
var index = 0u
while (true) {
val page = requestPage(index) ?: return
for (item in page.chunks) {
val itemMetadata = calculateItemMetadata(item)
repository.saveMetadata(item.id, itemMetadata)
}
if (!page.hasNext) {
return
}
index = page.index + 1u
}
}
この関数は、while
と for
によってネストされたループがあり、読みやすいとは言えません。そこで、以下のように内側のループを private 関数として抽出したとします。
fun saveAllItemMetadata() {
var index = 0u
while (true) {
val page = requestPage(index) ?: return
saveMetadataInPage(page)
if (!page.hasNext) {
return
}
index = page.index + 1u
}
}
private fun saveMetadataInPage(page: Page) {
for (item in page.chunks) {
val itemMetadata = calculateItemMetadata(item)
repository.saveMetadata(item.id, itemMetadata)
}
}
この抽出によって、読みやすさが劇的に改善したとは言えません。より良いリファクタリングの方法はあるでしょうか?
関係から森を見る
先述のリファクタリングで読みやすさがあまり改善されていない理由の 1 つに、関数の境界と意味のまとまりの境界が一致していないことが挙げられます。saveAllItemMetadata
が行うことをまとめると、以下の 2 つで表せます。
- すべての
Item
を取得する - その
Item
のメタデータを保存する
saveMetadataInPage
を抽出することによって、「すべての Item
を取得する」というコードが 2 箇所に分割されています。その結果、「saveAllItemMetadata
が何をするか」がかえって分かりづらくなりました。単に抽出しやすいという理由で、内部の構造をそのままに抜き出してしまうと、このような状 況を引き起こしかねません。リファクタリングで抽出を行う際は、 抽出のしやすさではなく、どのような改善をもたらすかに着目すべき です。そのためにも、抽出を行う際には 「コードが何をするか」の意味のまとまり に注意する必要があります。
今回の場合、内側のループを抽出するよりも Item
の列を取得する関数を作成するとよいでしょう。Kotlin ならば、Sequence<Item>
や Iterator<Item>
といったクラスで表現することができます。以下の requestItemSequence
では、Item
の列を Sequence<Item>
で返しています。
fun saveAllItemMetadata() {
for (item in requestItemSequence()) {
val itemMetadata = calculateItemMetadata(item)
repository.saveMetadata(item.id, itemMetadata)
}
}
fun requestItemSequence(): Sequence<Item> = sequence {
var page: Page? = requestPage(0u)
while (page != null) {
yieldAll(page.chunks)
page = if (page.hasNext) requestPage(page.index + 1u) else null
}
}
このようにすることで、ネストされたループを単一のループと見せかけることができます。その結果、requestItemSequence
の内容を理解しなくても、saveAllItemMetadata
を読むだけで「各 Item
についてメタデータを作り、保存する」という流れが分かるようになりました。
このリファクタリングの考え方は、ループのネストのみならず、条件分岐のネストやデータ構造のネストにも適用できます。抽出を行う際は、 既存の構造を残したほうがよいのか、それとも再構成したほうがよいのかを検討しましょう。
一言まとめ
抽出を行う際は、「コードが何をするか」の意味のまとまりに注意する。
キーワード: extraction
, boundaries of concept
, nested loop