こんにちは。コミュニケーションアプリ「LINE」のモバイルクライアントを開発している石川です。
この記事は、毎週木曜の定期連載 "Weekly Report" 共有の第 34 回です。 LINEヤフー社内には、高い開発生産性を維持するための Review Committee という活動があります。ここで集まった知見を、Weekly Report と称して毎週社内に共有しており、その一部を本ブログ上でも公開しています。(Weekly Report の詳細については、過去の記事一覧を参照してください)
三十六計分けるに如かず
メッセージを送受信するサービスの UI レイアウトを実装しているとします。このレイアウト内では、様々な「ボタン」を実装する必要があります (例: メッセージ送信ボタン、プロフィール編集ボタン、...)。以下のコードは、メッセージ送信ボタン SendMessageButton
とプロフィール編集ボタン ProfileStatusEditToggleButton
の実装です。これらのボタンは、クリックされたときに onClicked
が呼ばれます。
class SendMessageButton(
private val messageTextForm: TextForm,
private val useCase: MessageServiceUseCase
) : MessageServiceButton() {
override fun onClicked(button: Button) {
val message = Message(messageTextForm.text, ...)
val action = MessageServiceAction.SendMessage(message)
useCase.executeServiceAction(action)
messageTextForm.clear()
}
}
class ProfileStatusEditToggleButton(
private val statusTextForm: TextForm,
private val useCase: MessageServiceUseCase
) : MessageServiceButton() {
override fun onClicked(button: Button) {
val isEditable = !button.isChecked
button.isChecked = isEditable
statusTextForm.isEditable = isEditable
if (!isEditable) {
val statusText = statusTextForm.text
val action = MessageServiceAction.UpdateProfileStatus(statusText)
useCase.executeServiceAction(action)
}
}
}
実際にボタンがクリックされたとき動作は、MessageServiceAction
によって指定され、その値によって MessageServiceUseCase
が処理します。このようなクラス構成にしたのは、MessageServiceUseCase
と MessageServiceAction
のレイヤによってレポジトリを各 Button
が直接操作させないことを意図してのことでした。
class MessageServiceUseCase(
private val messageRepository: MessageRepository,
private val profileRepository: ProfileRepository,
...
) {
fun executeServiceAction(action: MessageServiceAction) {
when (action) {
is MessageServiceAction.SendMessage -> action(messageRepository)
is MessageServiceAction.UpdateProfileStatus -> action(profileRepository)
...
}
}
}
sealed interface MessageServiceAction {
class SendMessage(
private val message: Message
) : MessageServiceAction {
operator fun invoke(messageRepository: MessageRepository) {
// ... actual logic
}
}
class UpdateProfileStatus(
private val statusText: String
) : MessageServiceAction {
operator fun invoke(profileRepository: ProfileRepository) {
// ... actual logic
}
}
...
}
しかし、このコードには問題点がいつくかあります。それは何でしょうか。
Strategy not to craft
大きな問題としては以下の 3 つがあります。
- 条件の統合と再分岐: 各
Button
はMessageServiceAction
を指定することで動作を定義しています。その action は一度MessageServiceUseCase.executeServiceAction
で 1 つのフローに集められているのにもかかわらず、その中で再分岐しています。このような構造により、動作の流れが追いにくくなっているのが問題です。例えば、SendMessage.invoke
がどのような条件で呼ばれるかを確認するためには、すべてのexecuteServiceAction
の呼び出し元のコードを読む必要があります。 - 不要な sealed class:
sealed class
はダウンキャストでの網羅性を保証できる便利な言語機能ですが、今回の場合は条件分岐を整理することで、sealed class
を使わずに実装できます。他の言語においても、不要な associated value や直和型などは避けるべきでしょう。 - 不完全なロジックのレイヤ分け:
MessageServiceAction
はButton
からRepository
を隠蔽する役割を持っているのにもかかわらず、invoke
を自由に呼び出せてしまいます。そのため、間違った使い方をしてしまっても、それに気が付きにくいことが問題です。
これらの問題は、不適切な Strategy pattern の使用によって引き起こされています。まず、どのロジックを使うのかが静的に決まる場合は、Strategy pattern は不要 です。今回の場合は、MessageServiceUseCase
の関数を単純に分割するだけで十分でしょう。
class MessageServiceUseCase(
private val messageRepository: MessageRepository,
private val profileRepository: ProfileRepository,
...
) {
fun sendMessage(message: Message) {
// ... actual logic
// Use `messageRepository` here
}
fun updateProfileStatus(statusText: String) {
// ... actual logic
// Use `profileRepository` here
}
}
class SendMessageButton(
...
) : MessageServiceButton() {
override fun onClicked(button: Button) {
val message = Message(messageTextForm.text, ...)
useCase.sendMessage(message)
messageTextForm.clear()
}
}
class ProfileStatusEditToggleButton(
...
) : MessageServiceButton() {
override fun onClicked(button: Button) {
val isEditable = !button.isChecked
button.isChecked = isEditable
statusTextForm.isEditable = isEditable
if (!isEditable) {
useCase.updateProfileStatus(statusTextForm.text)
}
}
}
仮に Strategy pattern が必要になった場合でも、網羅性を保証する意味では Strategy クラスを sealed class
にする必要はありません。Strategy pattern は動的にロジックを決める機構 (動的ディスパッチなど) を使って実装されるため、ロジックを決める分岐は必要ないはずだからです。(もちろん、新たな Strategy を外部で定義できなくするという意味は、sealed class
は有効です。)
一言まとめ
どのロジックを使うのかが静的に決まる場合は、Strategy pattern の使用を避ける。
キーワード: strategy pattern
, conditional branch
, sealed class