-
-
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
Add Enum.valid?
#5716
Add Enum.valid?
#5716
Conversation
I'm fine with this, but obviously we can't rely on this method in Codegen. |
I don't intend to use this method in codegen entirely. But this method provides the way to distinguish an invalid (undefined) value from unhandled values in |
src/enum.cr
Outdated
# an instance method `valid?` is defined by the language when a user | ||
# defines an enum member named `Valid`. | ||
def self.valid?(value : self) : Bool | ||
from_value?(value.to_i) ? true : false |
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.
you could skip ternary expression: !!from_value?(value.to_i)
yields the same result.
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.
It is preferred. Thank you.
b237e1f
to
5fbebe9
Compare
Is it still useful for "the exhaustiveness checking for enums" ? |
@makenowjust what is the status on this? |
Is this just waiting for a review? |
If @makenowjust isn't working on this, it should be adopted or closed for the sake of cleaning up |
@barachy I will tackle with this issue tomorrow. This day is free to me. Sorry for slow resoponse 🙇♂️ |
It could be an instance method if the |
5fbebe9
to
1333c9c
Compare
Rebase to master and resolve request changes are done. |
SpecEnum.valid?(SpecEnum.new(3i8)).should be_false | ||
end | ||
|
||
it "for flags enum" 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.
should the following be added here?
SpecEnumFlags.valid?(SpecEnumFlags::None).should be_true
There is an existing spec for SpecEnumFlags.from_value(0).should eq(SpecEnumFlags::None)
.
Other than that, is GTG
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.
I also added SpecEnumFlags::All
case.
A valid enum value means that `ThisEnum.from_value?(value)` returns the `ThisEnum` member, and an invalid enum value means that it returns `nil`. This is implemented as the such way. The reason that this is implemented as a class method is explained in NOTE section of the document comment.
Co-Authored-By: Johannes Müller <johannes.mueller@smj-fulda.org>
Co-Authored-By: Johannes Müller <johannes.mueller@smj-fulda.org>
740cb52
to
cc1dc4b
Compare
A valid enum value means that `ThisEnum.from_value?(value)` returns the `ThisEnum` member, and an invalid enum value means that it returns `nil`. This is implemented as the such way. The reason that this is implemented as a class method is explained in NOTE section of the document comment.
As the first step to establish the exhaustiveness checking for enums, I want to define what enum value is valid or invalid. In this PR, a valid enum value means that
ThisEnum.from_value?(value)
returns theThisEnum
member, and an invalid enum value means that it returnsnil
. It makes sense to me, but maybe it is more natural if the relation betweenfrom_value?
andvalid?
is reversed.The reason that this is implemented as a class method is explained in NOTE section of the document comment.