こんにちは。コミュニケーションアプリ「LINE」のモバイルクライアントを開発している石川です。
この記事は、毎週木曜の定期連載 "Weekly Report" 共有の第 8 回です。Weekly Report については、第 1 回の記事を参照してください。
実像と虚像
以下の UserNameListLoader は、apiClient を通じて取得したユーザの ID と名前を管理します。取得するデータは、一度にすべて取得できるわけではなく、「チャンク」に区切られて得られます。
ここで議論を単純にするため、以下の仮定を導入します。
- このコードは特定のスレッドでしか動かない。 (スレッド安全性については考慮しなくて良い。)
- 取得できるデータは変化せず、チャンクは
UserIdでソートされている。（時間経過によるチャンクの不整合は考慮しなくて良い。）
class UserNameMapLoader(private val apiClient: ApiClient) {
private val userNameMap: MutableMap<UserId, String> = mutableMapOf()
private var lastChunkIndex: Int = -1
val loadedUserNameMap: Map<UserId, String>
get() = userNameMap
val loadedUserIds: Set<UserId>
get() = userNameMap.keys
val decoratedUserNameMap: Map<UserId, String>
get() = userNameMap.mapValues{ (_, value) -> "**$value**")
...
fun loadNextChunk() {
val response = apiClient.requestChunk(lastChunkIndex + 1) as? Success
?: return
userNameMap += response.chunk.toMapEntries()
lastPageIndex++
}
}このコードに改善するべき点はありますか？
石は勝手に転がる
もし、以下のようなコードを実行した場合、userNameMap とdecoratedUserNameMap の値の整合性が取れていないように見えてしまいます。
val userNameMap = loader.loadedUserNameMap
val userIds = loader.loadedUserIds
val decoratedUserNameMap = loader.decoratedUserNameMap
loadNextChunk()
println(userNameMap) // Shows {3=Alice, 5=Bob, 7=Charlie, 11=Dave}
println(userIds) // Shows [3, 5, 7, 11]
println(decoratedUserNameMap) // Shows {3=**Alice**, 5=**Bob**}3 つのローカル変数は Map か Set であるため、全て 読み取り専用 (unmodifiable/readonly) です。しかし、可変性 (mutability) について違いがあります。
loadedUserNameMap: オリジナルのMapの参照。loaderによって変化しうる。loadedUserIds: オリジナルのMapのキーの ビュー (view)。loaderによって変化しうる。decoratedUserNameMap: プロパティアクセス時に作成されたMapのコピー。いかなるloaderの操作によっても変化しない。
この問題を解決するにはいくつかのオプションがあります。
Option 1: 要素のアクセスだけを許可する
Map や Set を返さず、個々の UserId に対して名前を返すようにすることで、その後の更新に注意しなくて良くなります。
class UserNameMapLoader(private val apiClient: ApiClient) {
private val userNameMap: MutableMap<UserId, String> = mutableMapOf()
fun getName(userId: UserId): String? = userNameMap[userId]
...ただし、この実装を行うためには UserId のリストを別途管理する必要があり、loadNextChunk() を loadNamesFor(userIds: List<UserId>) のように変える必要があります。
Option 2: コピーだけを返す
場合によっては、後続の処理のためにコレクションを返したくなるときもあります。その場合は、必ずコピーを返すようにするのも一つの手段です。
class UserNameMapLoader(...) {
private val userNameMap: MutableMap<UserId, String> = mutableMapOf()
fun createUserNameMapSnapshot(): Map<UserId, String> =
userNameMap.toMap() // `toMap` is to create a copy of the map.
...関数名に "create"/"snapshot"/"copy"/"clone"/"newInstance" 等を加えることで、コピーが返されることを明確に示せます。また、toMap や toList に対してコメントを書くことて、他の開発者が誤ってコピーを行うコードを削除することを防げます。
ただし、この方法はパフォーマンスに影響を及ぼす可能性があるので、注意が必要です。（Swift の Array のようなコピーオンライトの仕組みがあるならば、それを利用すると良いでしょう。）
Option 3: 直接的な参照だけを返す
もし、他のオプションを適用できない場合は、オリジナルのコレクションへの参照をそのまま返すことが必要になる時もあります。ただし、（たとえシングルスレッドでも）コレクションの処理中、そのコレクションが変化しうる点に注意が必要です。そのため、この注意点についてドキュメンテーションで言及しておくべきです。
class UserNameMapLoader(...) {
private val userNameMap: MutableMap<UserId, String> = mutableMapOf()
/**
* A map with UserId as the key and usernames.
*
* This map is read-only, but the contents can be changed through
* operations on this `UserNameListLoader`.
*/
val userNameMap: Map<UserId, String> = mutableUserNameMap
...このオプションを採用するためには、コレクションを走査する関数 (filter/map/flatMap...) は呼び出し元で行われる必要があります。もしロジックの共通化が必要な場合は、呼び出し先のメンバとして実装する代わりにユーティリティ関数を実装するなど、依存関係を整理する工夫をしてください。
一言まとめ
関数の戻り値について、取得後に変化しうるかどうかを明確にする。
キーワード: view of collection, copied value, mutability
