CODEWITHSUNDEEP
kotlinswitch-statementrefactoring
arianhf
arianhf

Reputation: 153

Should I get rid of big switch case?

I have a factory which includes many HTML attribute generators which returns one of them based on the type of attribute, so I wanted to see if there is a better way of doing this.

class AttributeHtmlGeneratorFactory {
    fun create(property: String): AttributeHtmlGenerator {
        when (property) {
            "animation" -> {
                return AnimationHtmlGenerator()
            }
            ...
            "left", "top" -> {
                return PositionHtmlGenerator()
            }
            ...
            "scaleX" , "scaleY", ... , "direction" -> {
                return UnusedAttributesHtmlGenerator()
            }

this when switch has like 20 switch cases in it.

this is the interface which all these classes are using

interface AttributeHtmlGenerator {
    fun generateHtml(member: KProperty1<HtmlComponentDataModel, *>, component: HtmlComponentDataModel ): String
}

and this is where and how I'm using all of these:

        var result = ""

        HtmlComponentDataModel::class.memberProperties.forEach { member ->
            val generator = AttributeHtmlGeneratorFactory().create(member.name)
            result = result.plus(generator.generateHtml(member, component))
        }

        return result

also, this is a simple implementation of the interface:

class ButtonFillHtmlGenerator : AttributeHtmlGenerator {
    override fun generateHtml(member: KProperty1<HtmlComponentDataModel, *>, component: HtmlComponentDataModel): String {
        var result = ""

        member.get(component)?.let {
            result = result.plus("background-color:${it};")
        }

        return result
    }
}

is there anyway to make this better?

Upvotes: 0

Views: 319

Answers (1)

Horațiu Udrea
Horațiu Udrea

Reputation: 1877

If you just want to reformat the when statement, I suggest you you do like this:

fun create(property: String): AttributeHtmlGenerator = when (property)
{
    "animation" -> AnimationHtmlGenerator()
    "left", "top" -> PositionHtmlGenerator()
    "scaleX", "scaleY", "direction" -> UnusedAttributesHtmlGenerator()
    else -> error("No generator found for property $property")
}

If you want to split this logic across modules, you would use a Map.

class AttributeHtmlGeneratorFactory {
    private val generatorMap = mutableMapOf<String, () -> AttributeHtmlGenerator>()

    init {
        assignGeneratorToProperties("animation") { AnimationHtmlGenerator() }
        assignGeneratorToProperties("left", "top") { PositionHtmlGenerator() }
    }

    fun create(property: String): AttributeHtmlGenerator {
        return generatorMap[property]?.invoke() ?: error("No generator found for property $property")
    }

    fun assignGeneratorToProperties(vararg properties: String, provider: () -> AttributeHtmlGenerator) {
        properties.forEach {
            generatorMap[it] = provider
        }
    }
}

This way you can call assignGeneratorToProperties in parts of the code and thus split the initialization logic.

Performance-wise, when/if-else statements are really performant when you have a few cases but a HashMap outperforms them for a lot of elements. You decide what to use depending on your case.

Upvotes: 2

Related Questions