The original article was published on November 28, 2024.
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.
Prune branches and wither roots
Let's say a service has three account types (Free, Business, Premium). Additionally, you want to switch the display of three UI elements (background, icon, text) according to the account type as follows.
| UI element \ account type | Free | Business | Premium |
|---|---|---|---|
| Background color | gray | blue | yellow |
| Icon image | 👤 | 👔 | 👑 |
| Type text | Free... | Business | Premium!! |
To implement this 2D table, you need to separate the code by either account type or UI element as follows.
Option 1 Cut vertically: Separate functions or code blocks for each account type. Set all UI elements within the separated code.
Option 2 Cut horizontally: Separate functions or code blocks for each UI element. Determine the value of the element according to the account type within the separated code.
From the perspective of readability and robustness, which option is preferable?
Confine branches
Option 2, which involves splitting the code by UI element, is often better. To explore the reasons, let's compare the implementations of Option 1 and 2. In both implementations, the separated code is extracted as auxiliary private functions.
Option 1
/**
* Updates UI to indicate a given account type.
*/
fun updateAccountTypeLayout(accountType: AccountType) {
when (accountType) {
AccountType.FREE -> updateLayoutForFree()
AccountType.BUSINESS -> updateLayoutForBusiness()
AccountType.PREMIUM -> updateLayoutForPremium()
}
}
private fun updateLayoutForFree() {
backgroundUiElement.color = Color.GRAY
accountTypeIconUiElement.image = imageProvider.get(FREE_IMAGE_ID)
accountTypeTextUiElement.text = "Free..."
}
private fun updateLayoutForBusiness() {
backgroundUiElement.color = Color.BLUE
accountTypeIconUiElement.image = imageProvider.get(BUSINESS_IMAGE_ID)
accountTypeTextUiElement.text = "Business"
}
private fun updateLayoutForPremium() {
backgroundUiElement.color = Color.YELLOW
accountTypeIconUiElement.image = imageProvider.get(PREMIUM_IMAGE_ID)
accountTypeTextUiElement.text = "Premium!!"
}Option 2
/**
* Updates UI (background color, icon, and text) to indicate
* a given account type.
*/
fun updateAccountTypeLayout(accountType: AccountType) {
updateBackgroundColor(accountType)
updateIconImage(accountType)
updateTypeText(accountType)
}
private fun updateBackgroundColor(accountType: AccountType) {
backgroundUiElement.color = when (accountType) {
AccountType.FREE -> Color.GRAY
AccountType.BUSINESS -> Color.BLUE
AccountType.PREMIUM -> Color.YELLOW
}
}
private fun updateIconImage(accountType: AccountType) {
val iconId = when (accountType) {
AccountType.FREE -> FREE_IMAGE_ID
AccountType.BUSINESS -> BUSINESS_IMAGE_ID
AccountType.PREMIUM -> PREMIUM_IMAGE_ID
}
accountTypeIconUiElement.image = imageProvider.get(iconId)
}
private fun updateTypeText(accountType: AccountType) {
accountTypeTextUiElement.text = when (accountType) {
AccountType.FREE -> "Free..."
AccountType.BUSINESS -> "Business"
AccountType.PREMIUM -> "Premium!!"
}
}Option 2 has the following two advantages:
- You can imagine the behavior by reading only the main function (
updateAccountTypeLayout) without reading the details of the auxiliary private functions: In Option 1, readingupdateAccountTypeLayoutonly tells you that "the UI update logic differs for each account type." In contrast, Option 2 makes it clear that "the background, icon, and text are updated." - When adding new UI elements or account types, you can ensure that all combinations are implemented: In Option 1, when adding a new account type
ULTIMATE, even if you forget to updateaccountTypeTextUiElementwithin the auxiliary function, it cannot be detected at compile time. Furthermore, when adding a new UI element, you must update all existing functions, but forgetting to do so cannot be detected either. In contrast, in Option 2, if there is an implementation omission in the combination of account type and UI element, it can be detected at compile time by thewhenexpression.
Additionally, for Option 2, further refactoring can be performed, such as optimizing the range of code extracted into auxiliary functions. In Option 2-a, the code with side effects is moved to updateAccountTypeLayout, making the auxiliary functions referentially transparent.
Option 2-a
fun updateAccountTypeLayout(accountType: AccountType) {
backgroundUiElement.color = getBackgroundColorInt(accountType)
val iconId = getIconId(accountType)
accountTypeIconUiElement.image = imageProvider.get(iconId)
accountTypeTextUiElement.text = getTypeText(accountType)
}
private fun getBackgroundColorInt(accountType: AccountType): Int = when (accountType) {
AccountType.FREE -> Color.GRAY
AccountType.BUSINESS -> Color.BLUE
AccountType.PREMIUM -> Color.YELLOW
}This can be further developed by consolidating multiple values into a single data model, as in Option 2-b.
Option 2-b
fun updateAccountTypeLayout(accountType: AccountType) {
backgroundUiElement.color = accountType.backgroundColorInt
accountTypeIconUiElement.image = imageProvider.get(accountType.iconId)
accountTypeTextUiElement.text = accountType.typeText
}
// Case 1: Data model by an enum class
enum class AccountType(
val backgroundColorInt: Int,
val iconId: Int,
val typeText: String
) {
FREE(Color.GRAY, FREE_IMAGE_ID, "Free..."),
...
}
// Case 2: Data model by normal class
class AccountTypeLayoutModel(
val backgroundColorInt: Int,
val iconId: Int,
val typeText: String
) {
companion object {
val FREE = AccountTypeLayoutModel(Color.GRAY, FREE_IMAGE_ID, "Free...")
...
}
}Regarding conditional branching, it is often better to reduce the scope of the branch rather than avoiding duplication of branches. Even if there are similar branches, you can prevent update omissions by ensuring coverage, such as using the when expression. Especially in large-scale services or applications, if you try to consolidate branches into one place, multiple modules or layers may be described within that branch. By reducing the scope of the branch, it becomes easier to confine responsibility to each module or layer.
When coverage cannot be guaranteed
Depending on the language used, such as many dynamically typed languages, there may be no conditional branching that can guarantee coverage like the when expression. Java also became able to guarantee coverage of enums with switch from version 14, and until then, it was necessary to use invalid values like null with default, fall back to default values, or use exceptions like IllegalArgumentException.
@Nullable
Integer getBackgroundColorInt(@NonNull AccountType accountType) {
switch(accountType) {
case FREE:
return Color.GRAY;
...
default:
return null;
}In such cases where coverage cannot be guaranteed in production code, it is good to write test code to confirm coverage. The following test ensures that there is no unknown accountType. If a new AccountType is added but the update to getBackgroundColorInt is forgotten, assertNotNull will fail.
@Test
public void testBackgroundColorCompleteness() {
for (AccountType type : AccountType.values()) {
assertNotNull(
testTarget.getBackgroundColorInt(type),
"`getBackgroundColorInt` implementation is missing for the type " + type);
}
}In a nutshell
It is often better to prioritize reducing the scope over avoiding duplication of conditional branches.
Keywords: conditional branch, completeness, extraction
The series will resume after the New Year.