-
-
Notifications
You must be signed in to change notification settings - Fork 263
add a built-in ArgBuilder for enums
#2874
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
base: series/3.x
Are you sure you want to change the base?
Conversation
| case BooleanValue(value) => Right(value) | ||
| case other => Left(InvalidInputArgument("Boolean", other)) | ||
| } | ||
| lazy val enum: ArgBuilder[String] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can also potentially provide something like this for Java enums:
implicit def enumArgBuilder[T <: java.lang.Enum[T]: ClassTag]: ArgBuilder[T] =
enum.map(v => java.lang.Enum.valueOf[T](summon[ClassTag[T]].runtimeClass.asInstanceOf[Class[T]], v))not entirely sure how to do that for Scala enums since scala.reflect.Enum doesn't provide valueOf (but I think actual subclasses do?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or maybe something like this:
implicit def enumArgBuilder[T <: java.lang.Enum[T]: ClassTag]: ArgBuilder[T] =
`enum`.flatMap { value =>
val cls = summon[ClassTag[T]].runtimeClass.asInstanceOf[Class[T]]
Try {
java.lang.Enum.valueOf[T](cls, value)
}.toEither.left.map { t =>
ExecutionError(s"'$value' is not a valid value of ${cls.getSimpleName}", innerThrowable = Some(t))
}
}| case BooleanValue(value) => Right(value) | ||
| case other => Left(InvalidInputArgument("Boolean", other)) | ||
| } | ||
| lazy val `enum`: ArgBuilder[String] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the goal to use this + map for enums?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's correct, that was the intention!
i guess maybe to make that clearer, it should be something like the following...?
private lazy val enumBuilder: ArgBuilder[String] = {
case EnumValue(value) => Right(value)
case other => Left(InvalidInputArgument("Enum", other))
}
def `enum`[A](f: T => A): ArgBuilder[A] = enumBuilder.map(f)
def `enum`[A](f: T => Either[ExecutionError, A]): ArgBuilder[A] = enumBuilder.flatMap(f)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be f: String => ..., right? Maybe we can call it enumString and drop the backticks.
I didn't make it
implicitlike all the otherArgBuilders because its type conflicts with the existingArgBuilder[String]