こんにちは。コミュニケーションアプリ「LINE」のモバイルクライアントを開発している石川です。
この記事は、毎週木曜の定期連載 "Weekly Report" 共有の第 40 回です。 LINEヤフー社内には、高い開発生産性を維持するための Review Committee という活動があります。ここで集まった知見を、Weekly Report と称して毎週社内に共有しており、その一部を本ブログ上でも公開しています。（Weekly Report の詳細については、過去の記事一覧を参照してください）
三関数寄れば文殊の知恵?
クエリパラメータなどの渡された文字列をパースし、そこから値を得る機能を実装していることを想定しましょう。
以下の FooBarStringParser は、"foo:..." や "bar:..." という文字列から、それぞれ「Foo ID」と「Bar エントリ」を取得するというユーティリティオブジェクトです。ただし、"foo:..." も "bar:..." も文字列フォーマットにいくつかのバージョンがあるとし、FooBarStringParser はすべてのバージョンを正しく処理する必要があるとします。
object FooBarStringParser {
private val FOO_V1_REGEX: Regex = """foo:(\d{3})""".toRegex()
private val FOO_V2_REGEX: Regex = """foo-v2:(\d{3,4})""".toRegex()
private val FOO_V3_REGEX: Regex = """foo-v3:(\d{3,8})""".toRegex()
private val BAR_V1_REGEX: Regex = """bar:(\w+),(\w+)""".toRegex()
private val BAR_V2_REGEX: Regex = """bar-v2:(\w+)-(\w+)""".toRegex()
private val BAR_V3_REGEX: Regex = """bar-v3:entry=(\w+),(\w+)""".toRegex()
private val BAR_V4_REGEX: Regex = """bar-v4:(?:.+;)?entry=(\w+),(\w+);?.*""".toRegex()
/** (snip) */
fun getFooId(text: String): Int? = findFooIdForRegex(text, FOO_V3_REGEX)
?: findFooIdForRegex(text, FOO_V2_REGEX)
?: findFooIdForRegex(text, FOO_V1_REGEX)
private fun findFooIdForRegex(text: String, regex: Regex): Int? {
val matchResult = regex.matchEntire(text) ?: return null
return matchResult.groupValues[1].toInt()
}
/** (snip) */
fun getBarEntry(text: String): BarEntry? = findBarEntryForRegex(text, BAR_V4_REGEX)
?: findBarEntryForRegex(text, BAR_V3_REGEX)
?: findBarEntryForRegex(text, BAR_V2_REGEX)
?: findBarEntryForRegex(text, BAR_V1_REGEX)
private fun findBarEntryForRegex(text: String, regex: Regex): BarEntry? {
val groups = regex.matchEntire(text)?.groupValues ?: return null
return BarEntry(groups[1], groups[2])
}
}
class BarEntry(val key: String, val value: String)このユーティリティオブジェクトに、なにか問題点はありますか？
寄った結果の混乱
関数 findFooIdForRegex の引数 regex にどのような正規表現が渡されるかは、呼び出し元を見ない限り分かりません。しかし、findFooIdForRegex の中には groupValues[1].toInt() というコードがあるため、正規表現のグループが整数の文字列であることを期待しています。regex には任意の Regex インスタンスを渡すことができるのにもかかわらず、正しく動作させるためには「foo ID」の正規表現を渡す必要があります。
この状況を言い換えると、「foo ID」の 文字列フォーマットの知識が getFooId とfindFooIdForRegex に分割されてしまっている と言えます。FooRegex(val regex: Regex) のような特別な Regex のモデルを作成することで、状況を緩和できるかもしれませんが、オーバーエンジニアリングになる可能性がある上、「知識が分割されている」という問題の解決にはなっていません。
この分割されてしまった知識を一か所に統合するためには、「foo ID」に特有のコードをすべて呼び出し元 getFooId に移動し、プライベート関数 findFooIdForRegex から正規表現の知識に依存しないように書き換えるのが良いでしょう。以下のコードは、その例です。
fun getFooId(text: String): Int? {
val groups = getMatchedGroups(text, FOO_V3_REGEX)
?: getMatchedGroups(text, FOO_V2_REGEX)
?: getMatchedGroups(text, FOO_V1_REGEX)
?: return null
return groups[1].toInt()
}
private fun getMatchedGroups(text: String, regex: Regex): List<String>? =
regex.matchEntire(text)?.groupValues 呼び出し先が特定の知識に依存しないように再構成することで、さらなるリファクタリングの発想ができるときもあります。例えば、getMatchedGroups を findFirstMatchedGroup という名前に変え、複数の Regex を受け取れるようにリファクタリングしても良いでしょう。
fun getFooId(text: String): Int? {
val groups = findFirstMatchedGroups(text, FOO_V3_REGEX, FOO_V2_REGEX, FOO_V1_REGEX) ?: return null
return groups[1].toInt()
}
private fun findFirstMatchedGroups(text: String, vararg regexes: Regex): List<String>? = regexes.asSequence()
.mapNotNull { regex -> regex.matchEntire(text)?.groupValues }
.firstOrNull()findFirstMatchedGroups はもはや「foo ID」固有の知識を持たないので、「bar エントリ」のロジックにも流用ができます。最終的には以下のように単純化ができます。
object FooBarStringParserV2 {
private val FOO_V1_REGEX: Regex = """foo:(\d{3})""".toRegex()
private val FOO_V2_REGEX: Regex = """foo-v2:(\d{3,4})""".toRegex()
private val FOO_V3_REGEX: Regex = """foo-v3:(\d{3,8})""".toRegex()
private val BAR_V1_REGEX: Regex = """bar:(\w+),(\w+)""".toRegex()
private val BAR_V2_REGEX: Regex = """bar-v2:(\w+)-(\w+)""".toRegex()
private val BAR_V3_REGEX: Regex = """bar-v3:entry=(\w+),(\w+)""".toRegex()
private val BAR_V4_REGEX: Regex = """bar-v4:(?:.+;)?entry=(\w+),(\w+);?.*""".toRegex()
/** (snip) */
fun getFooId(text: String): Int? {
val groups = findFirstMatchedGroups(text, FOO_V3_REGEX, FOO_V2_REGEX, FOO_V1_REGEX) ?: return null
return groups[1].toInt()
}
/** (snip) */
fun getBarEntry(text: String): BarEntry? {
val groups = findFirstMatchedGroups(text, BAR_V4_REGEX, BAR_V3_REGEX, BAR_V2_REGEX, BAR_V1_REGEX) ?: return null
return BarEntry(groups[1], groups[2])
}
/** (snip) */
private fun findFirstMatchedGroups(text: String, vararg regexes: Regex): List<String>? = regexes.asSequence()
.mapNotNull { regex -> regex.matchEntire(text)?.groupValues }
.firstOrNull()
}一般論として、プライベート関数としてロジックを抽出することは、呼び出し元の関数の抽象度を高く維持する意味でも重要です。しかし、仕様に関する知識を一か所に集めること も同じように重要でしょう。
一言まとめ
仕様に関する知識を一か所にまとめることを意識する。
キーワード: specification knowledge, implicit assumption, extraction