-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Enum implementation overrides custom toString
#7227
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
Comments
This might be working as intended (which doesn't mean it isn't an issue...). You example is overriding the |
Yeah it does make sense considering how the implementation is. |
Totally agree, I think having enum specified as a desugaring step is not really ideal... |
Ideal or not, that's how enums are currently specified. So far there is a better alternative. |
Could we fix this issue by desugaring parameter-less enums to case objects instead of vals? |
I don't think it is about vals vs case objects. Translation specifies that a case of an enum is an instance of an anonymous class extending the parent enum and overriding its |
Yes. vals and objects behave the same here. It makes no difference wether I write enum E {
case A
case B()
override def toString: String = "overriden"
println(E.A) // --> A
println(E.B()) // --> overridden It would be good if we could change everything to So, to fix this, we need a redesign of (this aspect of) enums, and an implementation. |
We should reconsider adding |
@anatoliykmetyuk Indeed. And make the toString |
productPrefix is now overriden using the enum constant's name and used in the by-name lookup in EnumValues. java based enum values are optimised so that productPrefix will forward to .name in the simple enum case, avoiding an extra field
productPrefix is now overriden using the enum constant's name and used in the by-name lookup in EnumValues. java based enum values are optimised so that productPrefix will forward to .name in the simple enum case, avoiding an extra field
productPrefix is now overriden using the enum constant's name and used in the by-name lookup in EnumValues. java based enum values are optimised so that productPrefix will forward to .name in the simple enum case, avoiding an extra field
my current idea is since |
I don't think that's a good idea. While reasonable from DRY and engineering perspective, it adds a mental load on the programmer's mind. Now, to properly use enums, we need to keep in mind that they are products. For new programmers, this can be an extra learning curve when learning enums. |
@anatoliykmetyuk Thanks for your advice, I've said in this comment why at least the |
as per #9549 (comment) I can add an |
I like |
productPrefix is now overriden using the enum constant's name and used in the by-name lookup in EnumValues. java based enum values are optimised so that productPrefix will forward to .name in the simple enum case, avoiding an extra field
productPrefix is now overriden using the enum constant's name and used in the by-name lookup in EnumValues. java based enum values are optimised so that productPrefix will forward to .name in the simple enum case, avoiding an extra field
…ostring fix #7227: allow custom toString on enum
When overriding
toString
in an enum, simple cases end up using their name instead of this new implementation.minimized code
expectation
result
Note that adding an argument to that
enum
case will return the right result again:This prints
overriden
as expected.This has something to do with the desugaring of enums, and the generation of a default
toString
method.The text was updated successfully, but these errors were encountered: