Skip to content
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

Better enum symbol casting documentation #706

Merged
merged 5 commits into from
Aug 29, 2023
Merged

Better enum symbol casting documentation #706

merged 5 commits into from
Aug 29, 2023

Conversation

nobodywasishere
Copy link
Contributor

@nobodywasishere nobodywasishere commented Aug 11, 2023

Closes #705

@netlify
Copy link

netlify bot commented Aug 11, 2023

Deploy Preview for crystal-book ready!

Name Link
🔨 Latest commit 4c6e9fc
🔍 Latest deploy log https://app.netlify.com/sites/crystal-book/deploys/64dabaa7aef31c00087b5356
😎 Deploy Preview https://deploy-preview-706--crystal-book.netlify.app/syntax_and_semantics/enum
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@nobodywasishere nobodywasishere changed the title Better enum symbol casting documentation (#705) Better enum symbol casting documentation Aug 11, 2023
@Andriamanitra
Copy link

I think we should keep the old "Usage" section and add a new section for the symbol-enum autocasting. With the removal of the old example it is no longer clear how to use enums with a case statement. Especially this part that got removed provided helpful information:

However, if the programmer makes a typo, say :reed, the error will only be caught at runtime, while attempting to use Color::Reed will result in a compile-time error.

The recommended thing to do is to use enums whenever possible, only use symbols for the internal implementation of an API, and avoid symbols for public APIs. But you are free to do what you want.

@nobodywasishere
Copy link
Contributor Author

I could add an example like:

case color
when Color::Red
  # red color
else
  # unknown color
end

But I think that should be an additional example, not included in the def paint code example for clarity. I did also add a link to how to use enums with case statements as defined on the case page, which provides a different example.

I don't think the sections you quoted relate to how to use a case statement, but to the benefits of using enums over symbols. Maybe it could fit but honestly don't know if it does tonally. Maybe it could be something like:

If a symbol parameter is being used to indicate flags or a set of options, it is recommended to use an enum instead. If the wrong symbol argument is used, enum symbol casting provides a compile-time error letting you know there's a problem. This is in contrast to using a using a symbol parameter which may not catch an error at all without manual value checking or guard statements, leading to issues downstream.

@straight-shoota
Copy link
Member

I think it's fine like this. The paragraph mentioned in #706 (comment) is not very relevant for the enum page.
It talks about the shortcomings of symbol. So it should probably better be placed on the symbol page.

docs/syntax_and_semantics/enum.md Outdated Show resolved Hide resolved
docs/syntax_and_semantics/enum.md Outdated Show resolved Hide resolved
docs/syntax_and_semantics/enum.md Outdated Show resolved Hide resolved
@straight-shoota straight-shoota merged commit f71aacd into crystal-lang:master Aug 29, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Symbol enum auto-casting
4 participants