What do you think of that file - having the Extens...

# codereview

[Richard]

What do you think of that file - having the Extension function right below the enum?

enum class ToggleValue {
    YES,
    NO,
    UNKNOWN
}

val ToggleValue.asBoolean: Boolean? // this one
    get() = when (this) {
        ToggleValue.YES -> true
        ToggleValue.NO -> false
        ToggleValue.UNKNOWN -> null
    }

val Boolean?.asToggleValue: ToggleValue
    get() = when (this) {
        true -> ToggleValue.YES
        false -> ToggleValue.NO
        null -> ToggleValue.UNKNOWN
    }

[Joffrey]

Why not make the Boolean a property of the enum?

[Klitos Kyriacou]

Not sure if a property is more appropriate than a function. The Kotlin API has asXxx() functions, so wouldn't it be more consistent to make it asBoolean()? Maybe there is already some precedence that makes properties idiomatic but I'm not aware of such.

[Joffrey]

I meant a property of the enum itself rather than a conversion extension. If it's an extension, then I agree a asXxx() function is the general convention. But in general with enums that represent some values, I like to keep the mapping in one place and not repeat it. Something like this:

enum class ToggleValue(val boolValue: Boolean?) {
    YES(boolValue = true),
    NO(boolValue = false),
    UNKNOWN(boolValue = null);
    
    companion object {
        fun of(value: Boolean?) = entries.first { it.boolValue == value }
    }
}

fun Boolean?.asToggleValue(): ToggleValue = ToggleValue.of(this)

The companion of() could also be inlined, and we could just keep the extension:

enum class ToggleValue(val boolValue: Boolean?) {
    YES(boolValue = true),
    NO(boolValue = false),
    UNKNOWN(boolValue = null),
}

fun Boolean?.asToggleValue(): ToggleValue = ToggleValue.entries.first { it.boolValue == this }

[Richard]

ask my colleague.

[Edgar Avuzi]

You might prefer using bidirectional Map

[Edgar Avuzi]

dependencies {
    implementation("org.apache.commons:commons-collections4:4.4") // Check latest version
}

And then

import org.apache.commons.collections4.bidimap.DualHashBidiMap

val toggleToBooleanMap = 
  DualHashBidiMap<ToggleValue, Boolean?>().apply {
    put(ToggleValue.YES, true)
    put(<http://ToggleValue.NO|ToggleValue.NO>, false)
    put(ToggleValue.UNKNOWN, null)
  }

// Reverse lookup
val booleanToToggleMap = toggleToBooleanMap.inverseBidiMap()

// Usage
val booleanValue = toggleToBooleanMap[ToggleValue.YES] // true
val toggleValue = booleanToToggleMap[false] // <http://ToggleValue.NO|ToggleValue.NO>

[Joffrey]

Such an overkill, though (IMO). It saves almost nothing but adds apache commons deps

[Adam S]

As someone who has seen enums with so many fields they are effectively databases I am sympathetic to anyone who would prefer enums have no fields.

[David Breneisen]

The enum itself is overkill imo, it’s functioning exactly the same as Boolean?
Unless this is just an example

[Klitos Kyriacou]

It could be overkill, but on the other hand it could be useful because it provides type safety where there may be other variables/parameters that are also Boolean? but mean different things.

[David Breneisen]

Can you give an example? Not sure I get it or where the type safety is gained

[Klitos Kyriacou]

Example:

enum class ToggleValue { as above }
enum class LedStatus { ON, OFF, UNKNOWN }
fun foo(switchPosition: ToggleValue, lightOn: LedStatus)

fun foo(YES, ON) // ok
fun foo(ON, YES) // compile error
fun foo(true, false) // prone to runtime logic errors if named parameters are not used

Basically the same type safety concepts that value classes are used for.

[Edgar Avuzi]

I once worked with a function that had five parameters, three of which were Booleans. I accidentally passed the values in the wrong order. And spent some time digging into that problem. This likely wouldn’t have happened if those Booleans were replaced with domain-specific types

[Joffrey]

I agree with this line of thought, but I would just like to add that named parameters mitigate this problem partially in Kotlin

[David Breneisen]

Hmm yeah I guess I’d just prefer using named parameters when there are more than a couple args

[David Breneisen]

It just seems like a lot if you’re going to create an enum for everything that would be a boolean parameter

[Klitos Kyriacou]

everything that would be a boolean parameter

You'd be surprised how few these are. Yes, your application may have hundreds of boolean variables and dozens of functions that take boolean parameters, but these will represent only a small set of distinct entity types.