Skip to content

Enum extends another enum #4961

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
skvithalani opened this issue Aug 18, 2018 · 5 comments
Closed

Enum extends another enum #4961

skvithalani opened this issue Aug 18, 2018 · 5 comments

Comments

@skvithalani
Copy link
Contributor

We are currently using Enumeratum library to have enums in our code base. In a particular use case, we have a requirement such as there is inheritance in the behavior of enums for e.g.

sealed abstract class FullAlarmSeverity private[alarm] (val level: Int, val latchable: Boolean) extends EnumEntry with Lowercase {

  /**
   * The name of SeverityLevels e.g. for Major severity level, the name will be represented as `major`
   */
  def name: String = entryName

  def >(otherSeverity: FullAlarmSeverity): Boolean = this.level > otherSeverity.level

  def max(otherSeverity: FullAlarmSeverity): FullAlarmSeverity = if (otherSeverity > this) otherSeverity else this

  def isHighRisk: Boolean = this.level > 0
}

object FullAlarmSeverity extends Enum[FullAlarmSeverity] {

  /**
   * Returns a sequence of all alarm severity
   */
  def values: IndexedSeq[FullAlarmSeverity] = findValues ++ AlarmSeverity.values

  case object Disconnected extends FullAlarmSeverity(4, false)
}

sealed abstract class AlarmSeverity private[alarm] (override val level: Int, override val latchable: Boolean)
    extends FullAlarmSeverity(level, latchable)

object AlarmSeverity extends Enum[AlarmSeverity] {

  /**
   * Returns a sequence of all alarm severity
   */
  def values: IndexedSeq[AlarmSeverity] = findValues

  case object Okay          extends AlarmSeverity(0, false)
  case object Warning       extends AlarmSeverity(1, true)
  case object Major         extends AlarmSeverity(2, true)
  case object Indeterminate extends AlarmSeverity(3, true)
  case object Critical      extends AlarmSeverity(5, true)

As one would have noticed in the above code, FullAlarmSeverity companion object has values manipulated to also append the AlarmSeverity values, so that Okay from AlarmSeverity can also be deserialized to FullAlarmSeverity.

Using enums instead of ADT gives us the advantage of using withName and values methods. But trying to achieve something similar with Dotty seems pretty difficult.

So, is there any way to do so in Dotty with Enum or is there any other pattern that can give us the same effect of inheritance along with the advantage of withName and values methods from Enumeratum ?

@gberlanga
Copy link

I think the enum is the same.
Check out this documentation. http://dotty.epfl.ch/docs/reference/enums/enums.html

@Blaisorblade
Copy link
Contributor

We don't seem to have tests for enums that extend enums; it seems to only work in part, and by accident (at least in 0.9.0-RC1). We only test that case classes can't extend enums.

object enumTest {
  enum Test {
    case A
    case B
    case C
  }

  enum Test2 extends Test {
    case D
  }

  def testUser(t: Test): Unit = {
    t match {
      case Test.A =>
      case Test.B =>
      case Test.C =>
      case Test2.D => // 
    }
  }
  def test2User(t: Test2): Unit = { 
    t match {
      case Test2.D => // 
    }
  }
  def main(args: Array[String]) = {
    println(Test.enumValues) // MapLike.DefaultValuesIterable(A, B, C)
    println(Test2.enumValues) // MapLike.DefaultValuesIterable(D)
  }
}

Here, a Test value can be an instance of Test2.D (like a FullAlarmSeverity above can be an Okay), but Test.A isn't an instance of Test2 (like Disconnected isn't an instance of AlarmSeverity).

We provide enumValues, enumValueNamed and enumValue, but as shown they don't account for enum inheritance :-(. Looking at the output of -Xprint:frontend (available at https://gist.github.com/Blaisorblade/5e2990220947c55caf1e2993121be7bf), overriding them doesn't seem immediate. Among other things, the generated values don't even get disjoint IDs:

      final case val A: playground.enumTest.Test =
        playground.enumTest.Test.$new(0, "A")
      final case val B: playground.enumTest.Test =
        playground.enumTest.Test.$new(1, "B")
      final case val C: playground.enumTest.Test =
        playground.enumTest.Test.$new(2, "C")
//...
      final case val D: playground.enumTest.Test2 =
        playground.enumTest.Test2.$new(0, "D")

@Blaisorblade
Copy link
Contributor

Consensus from our weekly meeting is, enums are not meant to support this — they're a special case of case classes. We should add a compiler error for enums extending enums.

Other encodings might be possible; for instance:

trait Prefix {
  type EnableD
  enum Test {
    case A
    case B
    case C
    case D(erased implicit ev: EnableD =:= Any)
  }
}
object NoD extends Prefix { type EnableD = Nothing }
object WithD extends Prefix { type EnableD = Any }

@skvithalani
Copy link
Contributor Author

skvithalani commented Aug 23, 2018

I think it is good to not allow enum extend another enum. So, now if I want to achieve something similar to my example in the first comment I can code as follows:

trait AA
object AA {
  def enumValues: Iterable[AA] = A.enumValues ++ B.enumValues
  def enumValueNamed = A.enumValueNamed ++ B.enumValueNamed
}

enum A extends AA {
  case A1
}

enum B extends AA {
  case B2
}

object Main {
  def main(args: Array[String]): Unit = {
    val values: Iterable[AA] = AA.enumValues
    val a1: AA = AA.enumValueNamed("A1")
    val b2: AA = AA.enumValueNamed("B2")
  }
}

Now A and B both are children of AA and the companion of AA gives the effect of giving both the A's and B's enum values as AA types.

The enum extends enum could give the illusion that parent enum has child enum's values. This could be confusing for a newbie of Dotty. But now with this above code, it is explicit that A and B are children of AA.

@Blaisorblade
Copy link
Contributor

Seems we're set. I opened a smaller issue (#5008) to actually forbid the scenario; I'm closing this one but let us know (even here) if you see anything else to do on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants