Skip to content
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

Many to one #175

Merged
merged 15 commits into from
Mar 14, 2018
9 changes: 5 additions & 4 deletions enumeratum-core/src/main/scala/enumeratum/Enum.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package enumeratum
import scala.language.experimental.macros

import scala.collection.immutable._
import scala.collection.breakOut

/**
* All the cool kids have their own Enumeration implementation, most of which try to
Expand Down Expand Up @@ -38,20 +39,20 @@ trait Enum[A <: EnumEntry] {
/**
* Map of [[A]] object names to [[A]]s
*/
lazy final val namesToValuesMap: Map[String, A] =
values.map(v => v.entryName -> v).toMap
lazy val namesToValuesMap: Map[String, A] =
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

I think we should probably modify these Maps to be in terms of namesToValuesMap as well (I'd like to keep them final for now)

lazy final val lowerCaseNamesToValuesMap: Map[String, A] =
values.map(v => v.entryName.toLowerCase -> v).toMap
/**
* Map of [[A]] object names in upper case to [[A]]s for case-insensitive comparison
*/
lazy final val upperCaseNameValuesToMap: Map[String, A] =
values.map(v => v.entryName.toUpperCase -> v).toMap

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify what you mean here?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, they're built in terms of values directly, whereas I think they should be built in terms of namesToValuesMap so that they can have the same values (in case the user overrides namesToValuesMap and wants to use functions that call lowerCaseNamesToValuesMap)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I got it.

values.map(v => v.entryName -> v)(breakOut)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of using breakOut here; I know what it does, but:

  1. It's not obvious to beginners how it works;
  2. It'll likely be gone with the new collections lib
  3. This is a cached value; the so gains are pretty minimal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tl;dr Removed breakOut.

breakOut is definitely complex since CanBuildFrom isn't exactly the most intuitive concept, and I know that its days are numbered in Scala collections--though it isn't deprecated and probably won't be for some time. It just won't be necessary with the new views.

Also, breakOut is a memory optimization to eliminate the need for an intermediate sequence--cached values or not. I worry that might matter when people use enums in "Big Data" (as with Apache Spark) scenarios, but it is definitely true that the sequences for most cases are so small that breakOut probably doesn't change the world.


/**
* Map of [[A]] object names in lower case to [[A]]s for case-insensitive comparison
*/
lazy final val lowerCaseNamesToValuesMap: Map[String, A] =
values.map(v => v.entryName.toLowerCase -> v).toMap
values.map(v => v.entryName.toLowerCase -> v)(breakOut)

/**
* Map of [[A]] object names in upper case to [[A]]s for case-insensitive comparison
*/
lazy final val upperCaseNameValuesToMap: Map[String, A] =
values.map(v => v.entryName.toUpperCase -> v).toMap
values.map(v => v.entryName.toUpperCase -> v)(breakOut)

/**
* Map of [[A]] to their index in the values sequence.
Expand Down
31 changes: 21 additions & 10 deletions enumeratum-core/src/main/scala/enumeratum/EnumEntry.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ object EnumEntry {
*
* http://stackoverflow.com/a/19832063/1814775
*/
private val regexp1: Pattern = Pattern.compile("([A-Z]+)([A-Z][a-z])")
private val regexp2: Pattern = Pattern.compile("([a-z\\d])([A-Z])")
private val regexp1: Pattern = Pattern.compile("([A-Z]+)([A-Z][a-z])")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't need the class you added, please revert these formatting changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

private val regexp2: Pattern = Pattern.compile("([a-z\\d])([A-Z])")
private val replacement: String = "$1_$2"

// Adapted from Lift's StringHelpers#snakify https://github.com/lift/framework/blob/a3075e0676d60861425281427aa5f57c02c3b0bc/core/util/src/main/scala/net/liftweb/util/StringHelpers.scala#L91
Expand Down Expand Up @@ -62,63 +62,71 @@ object EnumEntry {
* Stackable trait to convert the entryName to Capital_Snake_Case .
*/
trait CapitalSnakecase extends EnumEntry {
override def entryName: String = stableEntryName
override def entryName: String = stableEntryName

private[this] lazy val stableEntryName: String = camel2WordArray(super.entryName).mkString("_")
}

/**
* Stackable trait to convert the entryName to Capital-Hyphen-Case.
*/
trait CapitalHyphencase extends EnumEntry {
override def entryName: String = stableEntryName
override def entryName: String = stableEntryName

private[this] lazy val stableEntryName: String = camel2WordArray(super.entryName).mkString("-")
}

/**
* Stackable trait to convert the entryName to Capital.Dot.Case.
*/
trait CapitalDotcase extends EnumEntry {
override def entryName: String = stableEntryName
override def entryName: String = stableEntryName

private[this] lazy val stableEntryName: String = camel2WordArray(super.entryName).mkString(".")
}

/**
* Stackable trait to convert the entryName to Capital Words.
*/
trait CapitalWords extends EnumEntry {
override def entryName: String = stableEntryName
override def entryName: String = stableEntryName

private[this] lazy val stableEntryName: String = camel2WordArray(super.entryName).mkString(" ")
}

/**
* Stackable trait to convert the entryName to CamelCase.
*/
trait Camelcase extends EnumEntry {
override def entryName: String = stableEntryName
override def entryName: String = stableEntryName

private[this] lazy val stableEntryName: String = super.entryName.split("_+").map(s => capitalise(s.toLowerCase)).mkString
}

/**
* Stackable trait to convert the entryName to UPPERCASE.
*/
trait Uppercase extends EnumEntry {
override def entryName: String = stableEntryName
override def entryName: String = stableEntryName

private[this] lazy val stableEntryName: String = super.entryName.toUpperCase
}

/**
* Stackable trait to convert the entryName to lowercase.
*/
trait Lowercase extends EnumEntry {
override def entryName: String = stableEntryName
override def entryName: String = stableEntryName

private[this] lazy val stableEntryName: String = super.entryName.toLowerCase
}

/**
* Stackable trait to uncapitalise the first letter of the entryName.
*/
trait Uncapitalised extends EnumEntry {
override def entryName: String = stableEntryName
override def entryName: String = stableEntryName

private[this] lazy val stableEntryName: String = uncapitalise(super.entryName)
}

Expand Down Expand Up @@ -167,6 +175,9 @@ object EnumEntry {
*/
trait LowerCamelcase extends EnumEntry with Camelcase with Uncapitalised

abstract class MultiEnum(override val entryName: String,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed (at least not here); looks like something you want in your tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

val alternateNames: String*) extends EnumEntry

/**
* Helper implicit class that holds enrichment methods
*/
Expand Down
5 changes: 5 additions & 0 deletions enumeratum-core/src/test/scala/enumeratum/EnumSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,11 @@ class EnumSpec extends FunSpec with Matchers {
UncapitalisedEnum.withName("goodBye") shouldBe UncapitalisedEnum.GoodBye
UncapitalisedEnum.withName("SIKE") shouldBe UncapitalisedEnum.Sike
UncapitalisedEnum.withName("a") shouldBe UncapitalisedEnum.a

MultiEnum.withName("One") shouldBe MultiEnum.One
MultiEnum.withName("1") shouldBe MultiEnum.One
MultiEnum.withName("eins") shouldBe MultiEnum.One
MultiEnum.withName("Two") shouldBe MultiEnum.Two
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions enumeratum-core/src/test/scala/enumeratum/Models.scala
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,19 @@ object UncapitalisedEnum extends Enum[UncapitalisedEnum] {

}

case object MultiEnum extends Enum[MultiEnum] {
import scala.collection.breakOut
val values = findValues
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't do this because the type parameter passed to Enum must be a sealed trait.

Compilation error https://travis-ci.org/lloydmeta/enumeratum/jobs/343456716#L844

[error] /home/travis/build/lloydmeta/enumeratum/enumeratum-core/src/test/scala/enumeratum/Models.scala:232:16: You can only use findValues on sealed traits or classes
[error]   val values = findValues
[error]                ^

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.


override val namesToValuesMap: Map[String, MultiEnum] =
values.flatMap { n =>
(n.entryName -> n) +: n.alternateNames.map(name => name -> n)
}(breakOut)

case object One extends MultiEnum("one", "1", "eins")
case object Two extends MultiEnum("two", "2", "zwei")
}

object Wrapper {

sealed trait SmartEnum extends EnumEntry
Expand Down