Ordinal on entries about enumeratum HOT 15 CLOSED

@darkfrog26 neat ideas :)

I think getting an enum by index could probably be done by making a Enum subclass that guarantees the the val values is an IndexedSeq and exposes such a method.

Having a method on each enum value that returns a name could maybe be something similar, but as of right now would be the same as calling .toString on the enum value.

As for getting the ordinal of an enum value as a method ... that might be a bit tough to do without either reflection or macros because the enum value would need a handle on the companion object's values sequence.

What do you think?

from enumeratum.

It just seems like the more common functionality to enums you can provide and more consistent with Java's enum functionality, the more useful this library will be. Additionally, the fact that you rely on toString for the name of the enum is problematic since there are valid scenarios where you might want the toString to be overridden with a different value but will still want to be able to access the name.

from enumeratum.

It just seems like the more common functionality to enums you can provide and more consistent with Java's enum functionality, the more useful this library will be.

Agreed; I think we can definitely get closer with your suggestions. I think to get the ordinal of an enum, one can always use a method on the companion, so I think I'll do it like that.

Additionally, the fact that you rely on toString for the name of the enum is problematic since there are valid scenarios where you might want the toString to be overridden with a different value but will still want to be able to access the name.

Also a good point, this lib should probably use some kind of name method on enums for both serialising and deserialising. Perhaps this can be done by constricting Enum's type parameter to a type that has a name method. This will be a small breaking change so I'll need to bump a minor API version for this.

from enumeratum.

You're currently using a Set for findValues...is that going to be a problem for guaranteeing order?

For the moment in my code I'm doing this:

lazy val name = getClass.getSimpleName.substring(0, getClass.getSimpleName.length - 1)

It's hacky, but it works for the moment.

from enumeratum.

You're currently using a Set for findValues...is that going to be a problem for guaranteeing order?

It will, so perhaps it would make sense to introduce an OrderedEnum[A] extends Enum[A] where values is refined/defined to be an IndexedSeq[A] instead of just an Iterable[A].

BTW: the reason why findValues returns a Set is because there is no guarantee in the reflection API at compile time that the values will be found in the order that they are written.

For the moment in my code I'm doing this:
lazy val name = getClass.getSimpleName.substring(0, getClass.getSimpleName.length - 1)
It's hacky, but it works for the moment.

Cool, is this a name on each individual enum instance or on the companion object? I was thinking that I could force individual enums to extend from say EnumValue where there is a default name method that by default delegates to toString. That way it works the same as it is now by default but can be overridden if needed.

For example, things might look like this:

trait EnumValue {
  /**
    By default, delegates to .toString, which should be reasonable as lone as EnumValues are case   objects

    override if needed
  */
  def name: String = this.toString
}

// Now constrained to A's that are subclasses of EnumValue
trait Enum[A <: EnumValue] {
  // ... this remains the same
}

// New Enum companion type that allows you to get the indexOf a member value
trait OrderedEnum[A] extends  Enum[A <: EnumValue] { 

  /**
   Returns the index of the enum value. If the object given is not in the registered set of values 
   (e.g. not contained inside this companion object and thus not found by findValues), returns -1.
  */
  def indexOf(o: A): Int = values.indexOf(o)

  def values: IndexedSeq[A] // force this to be an IndexedSeq
}

from enumeratum.

I just replaced my own enum implementation (was using reflection and ASM) with enumeratum and ran into the same issue, but using ASM I was able to access the items in the correct order, but the code looked like this:

class MyEnum extends EnumEntry

object MyEnum extends Enumerated[MyEnum] {
  val First = new MyEnum
  val Second = new MyEnum
  val Third = new MyEnum
}

Another option in the case of vals being instantiated is the ability to have them self-add to a list since that guarantees proper ordering as well.

The 'name' code was on every entry.

from enumeratum.

I just replaced my own enum implementation (was using reflection and ASM) with enumeratum and ran into the same issue, but using ASM I was able to access the items in the correct order, but the code looked like this:

@darkfrog26 sorry, I'm unfamiliar with ASM. Do you have a sample somewhere I can take a look at? A quick google search seems to point to some kind of bytecode emitter..

Another option in the case of vals being instantiated is the ability to have them self-add to a list since that guarantees proper ordering as well.

True, this is an option I haven't investigated in detail.

from enumeratum.

Here is my original enum:

https://github.com/darkfrog26/powerscala/tree/2536c1a6b256bfe615b14a5b6289be13f32598b4/core/src/main/scala/org/powerscala/enum

It uses my reflection library in powerscala that leverages ASM (http://asm.ow2.org/asm50/javadoc/user/org/objectweb/asm/tree/ClassNode.html#fields) to get the fields in the proper order. However, the reason I've switched to your library is to avoid the use of runtime reflection especially since it causes serious problems in Android development.

from enumeratum.

I've released 1.2.0 that addresses the enum name suggestion.

I think the auto-indexing (and thus auto enum entry ordering) aspect of this issue deserves more thought though, as that part is hard to do right while using case object entries (for exhaustive match errors). For example, this is the best I've been able to find that manages to figure out all the edge cases and it is fairly complex.

from enumeratum.

Two things about your change:

1. I think 'name' would be better than 'entryName'. It would be one thing if EnumEntry were going to be mixed into existing classes, but realistically enums will almost always be their own objects.
2. Defaulting it to use toString doesn't really help. Providing an internal mechanism to determine the name would be a much better solution as overriding toString also means you have to create your own implementation of deriving the name or manually setting it for each entry.

Regarding that example, what is it missing that enumeratum provides?

from enumeratum.

1. I originally had just name, but thought about the fact that this is for power users and so wanted to reduce the chances of stomping on their use case. e.g. what if they wanted to use name for something else?
2. So, the idea would be to override entryName, not toString. At the moment, it defaults to .toString because it works reasonably well with case objects. What kind of mechanism did you have in mind?

Regarding that example, what is it missing that enumeratum provides?

To be honest, I think it's a solid implementation :) A couple things that Enumeratum provides

• Simplicity; most of the complexity in this lib is in the macro, and the macro is fairly simple conceptually (finds implementations of a type inside a scope) and in its implementation. I gave up trying to figure out what that example does and why.
• No usage of synchronized at runtime , which may help with performance and deadlocks prevention
• No usage of reflection at run time. This may also help with performance but it means Enumeratum is compatible with ScalaJS or, in your case Android development. It also means that you find out sooner (at compile time) if things have broken (the reflection API is experimental) in newer releases of Scala.

from enumeratum.

1. That's reasonable
2. My problem is that if someone overrides toString to be something entirely different then they also have to override entryName or the naming is messed up. If we could simply reproduce exactly how case object does toString without actually using toString that would be perfect.

I would assume the compile-time macros can iterate over the case objects in correct order. If we could do that then we can accomplish ordinals. Unfortunately I don't know macros very well or I'd try to help out with this.

from enumeratum.

My problem is that if someone overrides toString to be something entirely different then they also have to override entryName or the naming is messed up. If we could simply reproduce exactly how case object does toString without actually using toString that would be perfect.

Ah, ok. Yes, that is a totally reasonable concern. I was originally concerned about using getSimpleName because that throws odd errors depending on object nesting. I haven't explored this fully, but there's probably a need to use plain getClass.getName and then do proper parsing to get rid of oddities like the ones that show up here.

I would assume the compile-time macros can iterate over the case objects in correct order. If we could do that then we can accomplish ordinals. Unfortunately I don't know macros very well or I'd try to help out with this.

Good point. At your prompting, I went and reviewed the macro code; we were already using something that produced an ordered List of subclass symbols and I then turned it into a Set. I think this was maybe a carry-over from when I was using (the buggy) knownDirectSubclasses method which returns a Set.

I've created a branch here that switches the implementation to produce an IndexedSeq and it seems to work as planned. I should probably add some tests to make sure (maybe statistically ?) that the macro produces sequences in the proper written order :)

from enumeratum.

For what it's worth, ordinals is in :)

The naming issue should probably have it's own issue (or better yet, a PR :D ) so I'll close this one.

from enumeratum.

Great, thanks. I'll take a look ASAP.

from enumeratum.