KOTLIN: Refactoring a big return when conditional clause coupled with another one

Question

'Tis a design and technical code-wise question, so during some refactoring I was doing on a random project to bring it up to date, I encountered the following part, an infix function on a sealed class that was doing some sort of validation:

infix fun TaskState.allowsOperation(operation: TaskOperation): Boolean {
    return when (this) {
        TaskState.Scheduled -> {
            when (operation) {
                TaskOperation.Start -> true
                TaskOperation.Stop -> false
                TaskOperation.Pause -> false
                TaskOperation.Resume -> false
            }
        }
        TaskState.Running -> {
            when (operation) {
                TaskOperation.Start -> false
                TaskOperation.Stop -> true
                TaskOperation.Pause -> true
                TaskOperation.Resume -> false
            }
        }
        TaskState.Stopped -> {
            when (operation) {
                TaskOperation.Start -> false
                TaskOperation.Stop -> false
                TaskOperation.Pause -> false
                TaskOperation.Resume -> false
            }
        }
        TaskState.Suspended -> {
            when (operation) {
                TaskOperation.Start -> false
                TaskOperation.Stop -> true
                TaskOperation.Pause -> false
                TaskOperation.Resume -> true
            }
        }
        TaskState.Terminated -> {
            when (operation) {
                TaskOperation.Start -> true
                TaskOperation.Stop -> false
                TaskOperation.Pause -> false
                TaskOperation.Resume -> false
            }
        }
        TaskState.Created -> {
            when (operation) {
                TaskOperation.Start -> true
                TaskOperation.Stop -> false
                TaskOperation.Pause -> false
                TaskOperation.Resume -> false
            }
        }
    }
}

So basically, what's happening there was a sugar syntax addition that allowed code to be called like this in other parts:

if (task.state allowsOperation operation) doSomething()

However, I find that particular bunch of code to look ugly, and I could not come up with a reasonable another way of doing it, but at the same time to not produce too many changes.

The TaskState and TaskOperation are just some sealed classes:

sealed class TaskState {
    object Created : TaskState()
    object Scheduled : TaskState()
    object Running : TaskState()
    object Stopped : TaskState()
    object Terminated : TaskState()
}

sealed class TaskOperation {
    object Start : TaskOperation()
    object Stop : TaskOperation()
    object Pause : TaskOperation()
    object Resume : TaskOperation()
}

Do you have any suggestions on how to refactor this type of thingy?

Please add your code as text instead of images. It's a lot easier to read or copy it somewhere else that way. Also, if your code already works, and you just want to make it prettier, you should try asking this on Code Review Stack Exchange — user, Jun 15 '20 at 18:14
See [here](https://meta.stackoverflow.com/a/285557/10134209) for many reasons why posting images of code is a bad idea. — gidds, Jun 15 '20 at 22:16
Thanks for the tip, I've edited and put text, didn't think about it. :) Also, never actually even know about the Code Review Stack Exchange -> I'm going on it. Thanks! — g0dzax, Jun 16 '20 at 13:27

Tenfour04 · Answer 1 · 2020-06-15T19:16:56.050

There is no reason for these to be sealed classes, because every subclass is an object, and none of them have any distinct parameters. Just use enums, but you'll have to remove all the is keywords in places where when statements have been used on one of these.

You could simplify the amount of code by giving the TaskState class a parameter for allowable operations. This can replace everything you have above.

enum class TaskOperation {
    Start, Stop, Pause, Resume
}

enum class TaskState(private vararg val allowableOperations: TaskOperation) {
    Scheduled(TaskOperation.Start),
    Running(TaskOperation.Stop, TaskOperation.Pause),
    Stopped(),
    Suspended(TaskOperation.Stop, TaskOperation.Resume),
    Terminated(TaskOperation.Start);

    infix fun allowsOperation(operation: TaskOperation) = operation in allowableOperations
}

Also, this is opinion, but I think it's silly and bad for readability to use infix if the function doesn't join or compare two things.

ZSergei · Answer 2 · 2020-06-15T19:29:15.120

There are only 6 cases when the function can return true and 14 cases it could return false

In this case, we could handle only true branches like this:

fun TaskState.allowsOperation(operation: TaskOperation): Boolean {
    return when (this to operation) {
        TaskState.Scheduled to TaskOperation.Start -> true

        TaskState.Running to TaskOperation.Stop -> true
        TaskState.Running to TaskOperation.Pause -> true

        TaskState.Suspended to TaskOperation.Stop -> true
        TaskState.Suspended to TaskOperation.Resume -> true

        TaskState.Terminated to TaskOperation.Start -> true

        else -> false
    }
}

KOTLIN: Refactoring a big return when conditional clause coupled with another one

2 Answers2