Hello, I'm Munetoshi Ishikawa, a mobile client developer for the LINE messaging app.
This article is the latest installment of our weekly series "Improving code quality". For more information about the Weekly Report, please see the first article.
The only responsibility of assigning responsibility
Let's say you want to implement a "launch button" that launches either fireworks, a rocket, or a product. Additionally, this launch button needs to dynamically change what it launches when pressed.
The following LaunchButtonBinder takes the launch logic Launcher as a constructor argument. It registers an event listener in the init method to ensure the appropriate Launcher is executed when the button is pressed.
class LaunchButtonBinder(
button: ObservableButton,
private val fireworkLauncher: Launcher,
private val rocketLauncher: Launcher,
private val productLauncher: Launcher
) {
var launchMode: LaunchMode = LaunchMode.OFF
init {
button.observePush {
launchMode.launcherSelector(this).launch()
}
}
enum class LaunchMode(val launcherSelector: LaunchButtonBinder.() -> Launcher) {
OFF({ Launcher.NOP_LAUNCHER }),
FIREWORK(LaunchButtonBinder::fireworkLauncher),
ROCKET(LaunchButtonBinder::rocketLauncher),
PRODUCT(LaunchButtonBinder::productLauncher)
}
}This binder class is simple enough, but it can be seen as having two responsibilities: "binding the logic when the button is pressed" and "deciding which Launcher is active". Therefore, we changed it so that one binder class binds to one Launcher, and implemented the functionality to select the Launcher in a separate class called LaunchBinderSelector.
In this code, when an instance of LaunchBinderSelector is created, an instance of LaunchButtonBinder is created for each Launcher. Since each LaunchButtonBinder registers an event listener, there are a total of three listeners registered to the button. Additionally, to control which of the three listeners is executed, a property called LaunchButtonBinder.isEnabled was added.
class LaunchButtonBinder(
button: ObservableButton,
private val launcher: Launcher
) {
var isEnabled: Boolean = false
init {
button.observePush {
if (isEnabled) {
launcher.launch()
}
}
}
}
class LaunchBinderSelector(
button: ObservableButton,
fireworkLauncher: Launcher,
rocketLauncher: Launcher,
productLauncher: Launcher
) {
private val binders: Map<LaunchMode, LaunchButtonBinder> = mapOf(
LaunchMode.FIREWORK to LaunchButtonBinder(button, fireworkLauncher),
LaunchMode.ROCKET to LaunchButtonBinder(button, rocketLauncher),
LaunchMode.PRODUCT to LaunchButtonBinder(button, productLauncher)
)
fun setLaunchMode(newMode: LaunchMode) = LaunchMode.entries.forEach { mode ->
binders[mode]?.isEnabled = newMode == mode
}
enum class LaunchMode { OFF, FIREWORK, ROCKET, PRODUCT }
}This way, we separated the logic implementation for the button and the responsibility of the Launcher. However, new problems have arisen. What are they?
Passing the buck of responsibility
One problem is "the responsibility to ensure the constraints of the specification is dispersed".
In the previous code, there was a constraint that only one thing would be launched when the button was pressed. In the new code, this is achieved through the combination of forEach, isEnabled, and getBinder, but to confirm this is correct, you have to read all the code. This problem can be solved by adding a property to LaunchBinderSelector that indicates which Launcher is selected. However, this would result in duplicated state between the added property and LaunchButtonBinder.isEnabled.
Another problem is related to the concealment of details.
In the split code, the role of LaunchBinderSelector was supposed to be only to select the LaunchButtonBinder. However, to create an instance of LaunchButtonBinder, it must directly depend on the Launcher. To conceal the Launcher from LaunchBinderSelector, you might consider directly holding instances of LaunchButtonBinder.
class LaunchButtonBinder(
button: ObservableButton,
private val launchAction: () -> Unit,
) {
var isEnabled: Boolean = false
init {
button.observePush {
if (isEnabled) {
launchAction()
}
}
}
}
class LaunchBinderSelector(
private val fireworkBinder: LaunchButtonBinder,
private val rocketBinder: LaunchButtonBinder,
private val productBinder: LaunchButtonBinder,
) {
fun setLaunchMode(newMode: LaunchMode) = LaunchMode.entries.forEach { mode ->
getBinder(mode)?.isEnabled = newMode == mode
}
private fun getBinder(mode: LaunchMode): LaunchButtonBinder? = when (mode) {
LaunchMode.OFF -> null
LaunchMode.FIREWORK -> fireworkBinder
LaunchMode.ROCKET -> rocketBinder
LaunchMode.PRODUCT -> productBinder
}
enum class LaunchMode { OFF, FIREWORK, ROCKET, PRODUCT }
}However, this means that the caller must resolve all dependencies (Selector, Binder, Button, Launcher). As a result, the caller may become bloated (the so-called "god class"), or you may need to read many classes to grasp the overall picture (the so-called "ravioli code").
LaunchBinderSelector(
LaunchButtonBinder(button, fireworkLauncher),
LaunchButtonBinder(button, rocketLauncher),
LaunchButtonBinder(button, productLauncher)
)The responsibility of considering responsibility
The initial implementation of LaunchButtonBinder had the advantage of implementing the specification constraints in one place, so overall, it may not have been necessary to split the class further.
When splitting classes, it is essential to avoid focusing too much on the responsibility range and cohesion within individual classes, leading to the caller's responsibility becoming bloated, or worsening the dependencies and coupling between classes and modules.
Summary in a nutshell
Be aware that splitting responsibilities can complicate dependencies.
Keywords: single responsibility, dependency, implicit constraints