-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Case Enum when Symbol #6592
Comments
To make it work you need to define Symbol#===, case rewrites it the other way around. The reason why it doesn't work out of the box with symbol is because === is defined at Object to return nil, so a symbol already matches that signature and always fails. By the way, you can do |
I prefer foo = rand > 0.5 ? Foo::Bar : nil
case foo
when .bar? # undefined method 'bar?' for Nil
case foo
when :bar # ok
when nil # ok Could this code be included into Crystal? I mean, I could finally contribute myself: abstract struct Enum
def symbol
{% begin %}
case value
{% for member, index in @type.constants %}
when {{index}}
{{member.symbolize.underscore}}
{% end %}
end
{% end %}
end
end
struct Symbol
def ===(enum _enum : Enum)
self == _enum.symbol
end
end
enum Foo
Bar
end
foo = Foo::Bar
pp! :bar === foo
case foo
when :bar then puts "true"
else puts "false"
end
# => (:bar) === foo # => true
# => true |
I just tried it and that implementation, compiled with That said, using underscore might not be ideal. I know the I'd like to have this in the language, but all the above reasons say to me not to do it. In fact, I'd like to remove symbols from the language. They were blindly copied from Ruby and have no real purpose in Crystal, and they make modular compilation impossible, if one ever wants to implement that. |
Even tho' I love symbols (coming from Ruby background), in this case I'd agree with all of the points mentioned by @asterite (maybe aside of removing 'em, I'd be sad to see them go, unless the gain would be worth it - like modular compilation). Adding to that |
abstract struct Enum
def ===(symbol : Symbol)
{% raise "Cannot call #===(Symbol) on flag enum" if @type.has_attribute?("Flags") %}
{% begin %}
case value
{% for member, index in @type.constants %}
when {{index}}
{{member.symbolize.downcase}} == symbol || {{member.symbolize}} == symbol
{% end %}
end
{% end %}
end
end
struct Symbol
def ===(enum _enum : Enum)
_enum === self
end
end This solves issues mentioned by @asterite. Regarding to efficiency, I don't think creating symbols leads to drastic performance issues... Calling I'm sure that casing enums with symbols is very intuitive. |
I've benched it:
With
case foo
when nil then puts "nil"
when .bar? then puts "bar"
when .baz? then puts "baz"
else puts "unknown"
end With symbols - nice and clean: case foo
when :bar then puts "bar"
when :baz then puts "baz"
else puts "nil or unknown"
end Let me create a PR, please |
No, it doesn't. raising is not any sensible solution. It wasn't the only problem, lost compile-time checks are another one.
And I'm sure I'm seeing this for the first time as a feature-proposal so I reckon not that intuitive, btw what's not intuitive about |
I'm fine with adding this to the standard library. I would even add |
I could wrap my head around implementing such a functionality with flags. Regarding intuitivity - when casing, I'm usually asking the code whether this variable is something: case x
when Int32 # IS it Int32? Note, no question mark in expression
when String # IS it String? Note, no question mark in expression We're missing such functionality for enums. I can't ask if it is However, checking if it's a specific enum by it's full name (e.g. |
@vladfaust Quoting from the docs:
Your proposal will mix two concepts (enums vs symbols), blurring the line (type-safe-ness) between those.
Wrong, in this case the type is an |
I don't understand how this would blur anything. It's an alternative to I've stated my thoughts, though. If there is anyone other than @asterite (which is not in core team IIRC) to support this proposal, let me know and I'll create a PR. |
enum Foo
A
B
end
foo = Foo::A
# compile-time safety
case foo
when .c? then # compile-time error
end
# no safety
case foo
when :c then # ouch
end |
@Sija why "ouch"? This wouldn't lead to an exception, instead a developer checks for a possible |
@vladfaust Imagine you have code like this: enum Color
Red
Gray
end
color = ...
case color
when :gray
# do sometihng
end Now your company gets bought by a UK company and they tell you to not use "gray" but "grey". When you change the enum value from Given that in Crystal we care about providing a language that's type-safe, I think accepting the code suggested in this issue to allow comparing enums and symbols like that goes against the philosophy of the language. |
I tried to find alternative ways to |
Implementing this would also have added two ways of doing the exact same thing to crystal. This has all the same issues as aliases. If we implement this syntax, we should remove symbols so that we know that @asterite the first step to removing symbols is making namedtuples use string literals instead of symbols I guess? |
However, after #6074 a developer is able to enum Color
Red
Gray
end
def foo(_enum : Color= :red)
end
foo(:gray) Now your company gets bought by a UK company and they tell you to not use "gray" but "grey". When you change the enum value from Gray to Grey and compile the code... it does not compile! Perfect! So why not implemeting the same functionality for |
@vladfaust you'd have to remove symbols to do that. |
@vladfaust Good question! The reason is that it's easier to implement it when there's a restriction in the method argument that says it accepts an enum. Then the compiler can tell "the user is passing a symbol to a method that accepts an enum, I will apply this special rule". In a Actually... Maybe at the end of compilation the compiler can check it, and if the case expression is turns out to be an enum it could try to apply the rule... But then... I think we'll have to do the same thing when you use And maybe a variable can be a symbol in one instantiation, or an enum in another instantiation. You don't want compilation to fail in that case. The solution is to remove symbols, which are pretty useless in Crystal (because we have enums and immutable strings). Then comparing something to a symbol can only mean it's a shorthand for an enum name. |
If you need my opinion, I support that kind of change, it would not cause any harm to any of my projects, @asterite. And the idea of
seems like a feature of a clean language for me. |
Would like to add a comment about the type safety:
In TypeScript such bug would be caught, and it would highlighted all the places that needed to be updated, as compiler won't allow you to use P.S. Literal types |
It's true that there can be some mistakes the compiler can't warn you about. In Crystal, That being said, there are a few ideas to improve the scenario you described. Exhaustive case statement is going to come eventually (see #8424). That would catch the spelling error (assuming the case is supposed to cover all enum values). The future of plain symbols is still unsettled, but I think there's a strong case for removing them from the language, which allow to fit the rules for symbol literals strictly to referencing enum values. How exactly that could work is yet to be determined, though. Anyway, the type-safe way to implement a
Any spelling mistake would immediately result in a compiler error.
That's not true. Enums are way more versatile than literal types. You can't implement flags or assign specific values. |
Okay, looks like
Enum#===
doesn't work with symbols (although I thought that #6074 would allow so). No problem, let's extendEnum
with#===(Symbol)
, because as stated at docs,case
expands to===
internally:That's a kind of strange behavior 🤔
The text was updated successfully, but these errors were encountered: