Kotlin Code Smell 26 - if-else/when statements

First programming lesson: Control structures. Senior developer lesson: avoid them.

Problems

• Too many decisions together
• Coupling
• Duplicated code
• Violation of Open/Closed Principle.
• A new condition should not change the main algorithm.
• Nulls

Solutions

1. Polymorphism
2. Create hierarchies/compose objects following the Open closed principle.
3. Use State pattern to model transitions.
4. Use Strategy Pattern/Method Object to choose for branches.

Examples

• Discrete Values
• State transition
• Algorithm choice.

Sample Code

Wrong

fun convertToMp3(source: Path, mimeType: String) =
    when(mimeType) {
        "audio/mpeg" -> convertMpegToMp3(source)
        "audio/wav" -> convertWavToMp3(source)
        "audio/ogg" -> convertOggToMp3(source)
        // Lots of new if-else cases
        else -> throw IllegalArgumentException("Unknown mime type")
    }

fun convertMpegToMp3(source: Path) = println("Convert from mpeg")
fun convertWavToMp3(source: Path) = println("Convert from wav")
fun convertOggToMp3(source: Path) = println("Convert from ogg")

Right

val registeredConverters = mapOf(
    "audio/mpeg" to ::convertMpegToMp3,
    "audio/wav" to ::convertWavToMp3,
    "audio/ogg" to ::convertOggToMp3,
    // Lots of other converters
)

fun convertToMp3(source: Path, mimeType: String) =
    registeredConverters[mimeType]
        ?.let { converter -> converter(source) }
        ?: throw IllegalArgumentException("No converter found")

fun convertMpegToMp3(source: Path) = println("Convert from mpeg")
fun convertWavToMp3(source: Path) = println("Convert from wav")
fun convertOggToMp3(source: Path) = println("Convert from ogg")

After Further Refactoring (Using Objects)

interface Mp3Converter {
    fun convertToMp3(source: Path)
}

class ConvertMpegToMp3 : Mp3Converter {
    override fun convertToMp3(source: Path) =
        println("Convert from mpeg")
}

class ConvertWavToMp3 : Mp3Converter {
    override fun convertToMp3(source: Path) =
        println("Convert from wav")
}

class ConvertOggToMp3 : Mp3Converter {
    override fun convertToMp3(source: Path) =
        println("Convert from ogg")
}

// Many more converters

val registeredConverters = mapOf(
    "audio/mpeg" to ConvertMpegToMp3(),
    "audio/wav" to ConvertWavToMp3(),
    "audio/ogg" to ConvertOggToMp3(),
    // Lots of other converters
)

fun convertToMp3(source: Path, mimeType: String) =
    registeredConverters[mimeType]
        ?.convertToMp3(source)
        ?: throw IllegalArgumentException("No converter found")

Conclusion

Excessive use of if-else/when statements in Kotlin can lead to code smells, making the code difficult to maintain and understand. To avoid these issues, it is essential to utilize design patterns such as polymorphism, state patterns, and strategy patterns.

You can almost always replace the if-else/when statement with a map implementation.

---

I hope you enjoyed this journey and learned something new. If you want to stay updated with my latest thoughts and ideas, feel free to register for my newsletter. You can also find me on LinkedIn or Twitter. Let's stay connected and keep the conversation going!

---

Credits

• Code Smell 36 - Switch/case/elseif/else/if statements by Maxi Contieri

Comments

[Martin Morales]

What if convertToMp3() needs parameters of different data type?
for example:
fun convertMpegToMp3(source: Path, size: Int)
fun convertWavToMp3(source: Path, subPath: String)

[Yonatan Karp-Rudin]

What you might want to consider is such cases is to create a ConvertParameters abstract class, and add the required parameters for each of the instances - for example:

interface Mp3Converter {
    fun convertToMp3(parameters: ConvertParameters)
}

sealed class ConvertParameters(val source: Path)
class MpegConvertParameters(source: Path, val size: Int) : ConvertParameters(source)
class WavConvertParameters(source: Path, val subPath: String) : ConvertParameters(source)

class ConvertMpegToMp3 : Mp3Converter {
    override fun convertToMp3(parameters: ConvertParameters) {
        val params = parameters as? MpegConvertParameters
            ?: throw IllegalArgumentException("Wrong parameters type")

        println("Convert from mpeg: ${params.source} - ${params.size}")
    }

}

class ConvertWavToMp3 : Mp3Converter {
    override fun convertToMp3(parameters: ConvertParameters) {
        val params = parameters as? WavConvertParameters
            ?: throw IllegalArgumentException("Wrong parameters type")

        println("Convert from wav: ${params.source} - ${params.subPath}")
    }
}

class ConvertOggToMp3 : Mp3Converter {
    override fun convertToMp3(parameters: ConvertParameters) =
        println("Convert from ogg: ${parameters.source}")
}

// Many more converters

val registeredConverters = mapOf(
    "audio/mpeg" to ConvertMpegToMp3(),
    "audio/wav" to ConvertWavToMp3(),
    "audio/ogg" to ConvertOggToMp3(),
    // Lots of other converters
)

fun convertToMp3(parameters: ConvertParameters, mimeType: String) =
    registeredConverters[mimeType]
        ?.convertToMp3(parameters)
        ?: throw IllegalArgumentException("No converter found")`