Curious what people think of this pattern: ```when...

# announcements

[Rob Elliot]

Curious what people think of this pattern:

when (enumValue) {
  Enum1, Enum2 -> doSomethingImperative()
  else -> Unit
}

as an alternative to

val certainEnumConstants = setOf(Enum1, Enum2) // defined in an object, so allocated once for the life of the runtime - recent edit in light of confusion in thread
if (enumValue in certainEnumConstants) {
  doSomethingImperative()
}

I like the lack of need to define and name a val enums: Set<EnumType> in the first, but I don’t like the need for an else branch.

[Vampire]

Two question, why do you need certainEnumConstants?
This should be the same, shouldn't it?

if (enumValue in setOf(Enum1, Enum2)) {
  doSomethingImperative()
}

And why do you need an else, if the when is not used as an expression, it does not have to be exhaustive.

[natario1]

As a third option, you could add a boolean property or function in the enum class or as an extension to it.

[Rob Elliot]

1) Possibly me making an invalid assumption - I assume that I’m creating a new set on every call to the if statement if I inline certainEnumConstants, but that in a when check it doesn’t. Perhaps the compiler (or JIT?) is cleverer than that - I should check.
2) You’re right - it’s a warning rather than an error. I’ve got in the habit of treating warnings as errors. I can’t suppress that warning, irritatingly…

[mkrussel]

You could always convert the when to an if and then delete the else.

[Rob Elliot]

Yes, but I hate if (enumValue == Enum1 || enumValue == Enum2 || ...)

[Rob Elliot]

On 1) - perhaps the JIT can do something, but the kotlin compiler ends up with this Java:

if (SetsKt.setOf(new MyEnum[]{Enum1, Enum2}).contains(enumValue)) {
  this.doSomethingImperative();
}

so it’s potentially allocating an array and a set on each invocation.

[ephemient]

it's allocating every time, yes.

[ephemient]

when is fine, and as Vampire says, doesn't require else -> if you're not using the result as an expression. and if you decide to create a set, Java's EnumSet might be interesting (it is a bit mask of element ordinals underneath)

[Vampire]

Ah, the missing part of 1) was, that you define the certainEnumConstants somewhere else and then use it repeatedly.
Then it makes a difference of course.

[Nir]

I realize how silly of a thing it is, but I feel like the double indentation of when always tends to make me shy away from it for simple use cases

[Rob Elliot]

Yes, sorry, I should have made it clear certainEnumConstants was a constant on an object.

[Nir]

unless i knew performance would matter I guess my default personally would probably be to write if (enumValue in listOf(Enum1, Enum2))

[Nir]

actually, if you simply use sequenceOf

[Nir]

you avoid the allocation

[ephemient]

not really

[ephemient]

varargs allocate an array no matter what

[Nir]

ah, that's rather disappointing

[Nir]

it doesn't require an allocation in principle, unlike a listOf or setOf approach

[Nir]

should still be faster though I would expect

[Nir]

well, who knows

[ephemient]

not what I would expect, because Sequence.contains() must use the generic iterator() while ArrayList.contains() can just go to its backing array

[Nir]

if (enumValue in sequence { yield(Enum1); yield(Enum2) }) 😛

[ephemient]

that's definitely worse, bouncing in and out of coroutines

[Nir]

yeah, if you are creating the array either way because of varargs, then yes, I'd expect you're right. In principle, the lazy version is easier to optimize but I guess the specifics take precedence here

[ephemient]

one thing to note is that the whole Java ecosystem is built around the idea of "separate compilation". at runtime, any class can be substituted with different implementation than you compiled with. so if something depends on certain behavior external to the class being compiled (e.g. the behavior sequenceOf()::contains) then it's not a valid compile-time optimization. and in this case, it's too complex for the JIT to look into either.

[Nir]

Right. Useful in software engineering terms but it seems like it greatly limits your optimization opportunities; inlining is pretty much the Ur-optimization in AOT languages

[ephemient]

although Kotlin already allows for smart casting of data class fields that are defined in the same module, under the assumption that your runtime is picking up whole dependencies (modules) at a time, not individual classes, and it does assume certain stdlib behavior (e.g. a in b..c == a >= b && a <= c, and that creating the range object has no side effects and can be omitted)

[ephemient]

so in theory, it could be argued for the compiler to optimize listOf()::contains and sequenceOf()::contains to a chain of `==`/`&&`. but it's not currently the case

[ephemient]

for now, if you're looking just at overhead, enumValue in arrayOf(Enum1, Enum2) has less - it's a vararg array creation just like all the `listOf()`/`sequenceOf()`, but nothing more than that

[ephemient]

and EnumSet.of() has a number of overloads so you might not even be using varargs there

[Travis Romney]

Instead of creating a new set or a sequence you could just use the builtin values method of the enum.

if (enumVal in EnumType.values()) {
doSomething()
}

[ephemient]

sure, there's EnumType.values() or enumValues<EnumType>() (kotlin-style)... but it sounded like Rob wanted to act on just a subset of the enum values. (also it creates a new Array each time, for compatibility reasons)