こんにちは。コミュニケーションアプリ「LINE」のモバイルクライアントを開発している石川です。
この記事は、毎週木曜の定期連載 “Weekly Report” 共有の第 30 回です。Weekly Report については、第 1 回の記事を参照してください。
運命の赤い糸 (透明)
他のユーザーから送られた「メッセージ」を表示する機能を実装するとしましょう。以下の MessageModel と MessageContent は、そのような「メッセージ」を表現するデータモデルです。これらのデータモデルは、UI 以外のレイヤでも使われるため、UI のためだけに定義を変えるのは不適切である とします。
class MessageModel(
val content: MessageContent,
val timestampMillis: Long,
val senderUserId: UserId,
...
)
sealed class MessageContent {
data object Empty: MessageContent()
data class Error(val errorType: ErrorType): MessageContent()
data class Text(val text: String): MessageContent()
data class Event(val eventType: EventType): MessageContent()
}メッセージを表示する UI レイアウトは以下のように実装されています。(なお、error はランタイムエラーを投げる Kotlin の標準関数です。)
fun bindToViews(message: MessageModel) {
val isVisible = isContentValid(message.messageContent)
layoutContainer.isVisible = isVisible
if (!isVisible) {
return
}
timestampTextView.text = formatter.format(message.timestampMillis)
... // その他のビュー更新ロジック
contentTextView.text = getMessageText(message.content)
}
private fun isContentValid(content: MessageContent): Boolean = when (content) {
MessageContent.Empty,
is MessageContent.Error -> false
is MessageContent.TextMessage,
is MessageContent.EventMessage -> true
}
private fun getMessageText(content: MessageContent): String = when (content) {
MessageContent.Empty,
is MessageContent.Error -> error("Invalid message type as `messageText`")
is MessageContent.TextMessage -> content.text
is MessageContent.EventMessage -> content.eventType.toMessageString()
}このコードに何か問題点はありますか？
一蓮托生ならまとめよう
上記のコードでは、getMessageText の実装が暗黙的に isContentValid に関連しており、結果として以下のような問題が起きています。
isContentValidがtrueを返す場合のみgetMessageTextを呼べることを前提にしているが、それを違反してもランタイムエラーでしか気付けない。- タイプが追加・更新されたときに、
isContentValidとgetMessageTextの間で挙動の整合性が取れているかを確認する必要がある。
bindToViews の実装を見ただけでは、isContentValid と getMessageText の関連性に気づかないかもしれません。この関連性を見落としてしまうと、リファクタリングや機能拡張で getMessageText の使い方を間違えたときに、それに気付くのは難しいです。
このような 暗黙的に関連する関数がある場合は、関数を 1 つにまとめることで解消できることがあります。現状のコードでは isContentValid はフィルタとして、getMessageText は変換として実装されているのですが、その根底にあるロジックは共通しています。そこで、これらの 2 つの役割を getMessageText に統合してみましょう。
/**
* メッセージコンテント [content] を表示可能なテキスト表現にして返す。
* ただし、表示できないコンテントタイプ (error/empty) の場合は null を返す。
*/
private fun getMessageText(content: MessageContent): String? = when (content) {
MessageContent.Empty,
is MessageContent.Error -> null
is MessageContent.TextMessage -> content.text
is MessageContent.EventMessage -> content.eventType.toMessageString()
}呼び出し元は、以下のようになります。
fun bindToViews(message: MessageModel) {
val messageText = getMessageText(message.messageContent)
layoutContainer.isVisible = messageText != null
if (messageText == null) {
return
}
timestampTextView.text = formatter.format(message.timestampMillis)
... // その他のビュー更新ロジック
contentTextView.text = messageText
}今回は、無効なコンテントタイプを示すために null を使いましたが、この仕様は getMessageText が広く使われるようになると混乱を招くかも知れません。そのような場合は、改めて isContentValid を定義するとよいでしょう。ただしそのときは、 isContentValid と getMessageText の関連性を明示するような実装にすべき です。例えば、isContentValid は独立した関数としてではなく、getMessageText を使って実装できます。
private fun isContentValid(content: MessageContent): Boolean =
getMessageText(content) != null
/** ... */
private fun getMessageText(content: MessageContent): String? = when (content) {
MessageContent.Empty,
is MessageContent.Error -> null
is MessageContent.TextMessage -> content.text
is MessageContent.EventMessage -> content.eventType.toMessageString()
}このようにすることで、単独で getMessageText を使っても安全で、かつ、isContentValid と getMessageText 間の整合性を保証できるような実装になります。
一言まとめ
関数の間に暗黙 の関連性がある場合、関数を 1 つにまとめたり、関連性が明らかな実装に変えたりする。
キーワード: conditional branch, function precondition, implicit dependency