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

Remove not_nil! from Regex::MatchData docs #13120

Closed
devnote-dev opened this issue Feb 26, 2023 · 2 comments
Closed

Remove not_nil! from Regex::MatchData docs #13120

devnote-dev opened this issue Feb 26, 2023 · 2 comments

Comments

@devnote-dev
Copy link
Contributor

The documentation for Regex uses a lot of not_nil! in the code examples, and is one of only 4 pages to do this, but it should not be like this. It is generally recommended to not use this method where possible (or unless absolutely necessary, IMO) and the Regex docs should reflect that.

Most (if not all) of the examples are to do with the String#match method which is nilable, so I have created #13119 to address that, but another solution could be to recommend the conditional statement check syntax. For example, take Regex::MatchData#[] which has the following code example:

"Crystal".match(/r(?<ok>ys)/).not_nil!["ok"] # => "ys"
"Crystal".match(/r(?<ok>ys)/).not_nil!["ng"] # raises KeyError

This could easily be changed to:

if matches = "Crystal".match(/r(?<ok>ys)/)
  matches["ok"] # => "ys"
  matches["ng"] # raises KeyError
end

I believe that this is much clearer than the existing example and it effectively avoids using not_nil! thanks to the conditional logic.

WDYT?

@straight-shoota
Copy link
Member

If #match! gets implemented, I think that would be the most concise way to get rid of .not_nil!.
So let's wait for the resolution of #13119

@stufro
Copy link

stufro commented Jul 23, 2023

I was looking at picking this one up but now #13285 has changed all the documentation for Regex::MatchData

Can this issue be closed now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants