I initially thought it could be made much shorter, but then I started coding it and I realized there are many Kotlin/Java limitations I was unaware of. Namely Kotlin enum can't override compareTo
and things can get quite strange due to the ordering of initialization of enum elements. More on that later.
In Kotlin you can use defender in interiorTo()
instead of inferiorTo().contains(defender)
.
Always use the most general type possible. Here that means using Set
instead of List
since the ordering is irrelevant.
The if
-expression in Kotlin is different from the Java if
-statement because it returns a value. So you can simplify your code:
return if (defender in inferiorTo()) {
ComparisonResult.INFERIOR
} else if (defender in superiorTo()) {
ComparisonResult.SUPERIOR
} else {
ComparisonResult.NEUTRAL
}
Furthermore it is often cleaner to use Kotlin's when
expression rather than a series of if-else
:
return when (defender) {
in inferiorTo() -> ComparisonResult.INFERIOR
in superiorTo() -> ComparisonResult.SUPERIOR
else -> ComparisonResult.NEUTRAL
}
I don't like the global structure (Element.Type
and Element.ComparisonResult
) so much. I think there should only be an Element
enum with the five subtypes for clarity. And ComparisonResult
should not exist because it is a re-implementation of the Comparable
interface. There are already so many APIs that it's not a good idea to introduce new ones. And as a bonus of using existing APIs, there is a lot of code that already exists than can use those, not to mention that people already know how to use APIs such as Comparable
.
Unfortunately, as I wrote in the opening paragraph, Kotlin enum
implements Comparable
as final (not overridable), where the ordering is the order in which the enum elements are defined. However Kotlin also has sealed class
es which are nearly identical to enum
. I posted some code below implementing your code using a sealed class, and implementing Comparable
. I don't like my code so much however and it could probably be improved.
sealed class Element : Comparable<Element> {
override fun compareTo(other: Element): Int {
val relations = allRelations[this] ?: throw IllegalStateException("allRelations should cover all values.")
return when (other) {
in relations.inferiorTo -> -1
in relations.superiorTo -> 1
else -> 0
}
}
}
object NEUTRAL: Element()
object FIRE: Element()
object WATER: Element()
object WIND: Element()
object EARTH: Element()
val allRelations: Map<Element, Relations> = mapOf(
NEUTRAL to Relations(),
FIRE to Relations(superiorTo = EARTH, inferiorTo = WATER),
WATER to Relations(superiorTo = FIRE, inferiorTo = WIND),
WIND to Relations(superiorTo = WATER, inferiorTo = EARTH),
EARTH to Relations(superiorTo = WIND, inferiorTo = FIRE));
data class Relations(val superiorTo: Set<Element>,
val inferiorTo: Set<Element>) {
constructor() : this(setOf(), setOf()) {}
constructor(superiorTo: Element, inferiorTo: Element) :
this(superiorTo = setOf(superiorTo), inferiorTo = setOf(inferiorTo))
init {
if (!(superiorTo intersect inferiorTo).isEmpty())
throw IllegalArgumentException("Can't have an Element both superior and inferior.")
}
}
fun main(args: Array<String>) {
println(FIRE < WATER)
}
You can see at the very end that I am printing out FIRE < WATER
to show that in Kotlin the greater-than and such operators are overloaded to make use of Comparable
. The convention for Comparable
is that this.compareTo(other)
returns a negative number if this
is smaller than other
.
I tried to make a solution (either using enum
or a sealed class) where the superiorTo and inferiorTo would just be specified in the constructors, ie. FIRE(superiorTo = EARTH, inferiorTo = WATER)
. But I failed since there are some complex issues related to the order in which the objects are created. The worst part is that the code would compile, but then I would get some mysterious runtime error about null references. [EDIT: I updated IntelliJ and it seems this was fixed! For enum
there is now a compile time error message and sealed class
can now handle out of order references in the constructors. See EDIT below.]
My guest is that the same issue was encountered when developing the original Java code. It struck me as quite odd to create getter methods that create and return a list instead of just having fixed members. It would have been better to just declare those as members in the constructor, which is impossible as described above. I am not a big fan of commenting code everywhere, but I think in this case this should have been documented since it seems wrong if you don't know the issue behind the decision.
In my solution the relationships between the elements are stored in a separate data structure: allRelations
. I was forced to do this since I could not declare those relationships in the constructors, ie. FIRE(superiorTo = EARTH, inferiorTo = WATER)
. But one could argue that it is better to have the elements and their relationships separated. This is the "separation of concerns" principle.
EDIT:
After updating IntelliJ, it seems that Kotlin has evolved in the time it took me to write this post. I can now show an example where the relations are defined directly as constructor members:
sealed class Element(val superiorTo: Set<Element>,
val inferiorTo: Set<Element>) : Comparable<Element> {
constructor(): this(setOf(), setOf()) {}
constructor(superiorTo: Element, inferiorTo: Element) :
this(setOf(superiorTo), setOf(inferiorTo))
override operator fun compareTo(other: Element): Int {
return when (other) {
in inferiorTo -> -1
in superiorTo -> 1
else -> 0
}
}
}
object NEUTRAL: Element()
object FIRE: Element(superiorTo = EARTH, inferiorTo = WATER)
object WATER: Element(superiorTo = FIRE, inferiorTo = WIND)
object WIND: Element(superiorTo = WATER, inferiorTo = EARTH)
object EARTH: Element(superiorTo = WIND, inferiorTo = FIRE)
fun main(args: Array<String>) {
println(FIRE < WATER)
println(EARTH < WATER)
println(EARTH <= WATER)
}
This is much shorter and cleaner. However there is still the issue of deciding if the relationships should be declared separately from the elements ("separation of concerns").