Skip to content

Prefer String#include? and match? over =~#372

Merged
grosser merged 1 commit into
grosser:masterfrom
tagliala:chore/remove-regex-match
Apr 22, 2026
Merged

Prefer String#include? and match? over =~#372
grosser merged 1 commit into
grosser:masterfrom
tagliala:chore/remove-regex-match

Conversation

@tagliala
Copy link
Copy Markdown
Contributor

Using =~ to check for a plain substring is unnecessary: it allocates memory for $~ and related global variables on every call, even when the result is only used as a boolean.

Prefer String#include? for plain string checks. When a regexp is needed (e.g. in specs), prefer match?. Neither pollutes the global regexp state.

Also prefer match over =~ for more readable specs

Benchmarks:


Comparison (IPS):
            include?: 25395688.0 i/s
              match?: 19470868.8 i/s - 1.30x  (± 0.00) slower
                  =~: 10289823.3 i/s - 2.47x  (± 0.00) slower

Comparison (Memory):
              match?:          0 allocated
            include?:          0 allocated - same
                  =~:        168 allocated - Infx more

Using `=~` to check for a plain substring is unnecessary: it allocates
memory for `$~` and related global variables on every call, even when
the result is only used as a boolean.

Prefer `String#include?` for plain string checks. When a regexp is
needed (e.g. in specs), prefer `match?`. Neither pollutes the global
regexp state.

Also prefer `match` over `=~` for more readable specs

Benchmarks:
```

Comparison (IPS):
            include?: 25395688.0 i/s
              match?: 19470868.8 i/s - 1.30x  (± 0.00) slower
                  =~: 10289823.3 i/s - 2.47x  (± 0.00) slower

Comparison (Memory):
              match?:          0 allocated
            include?:          0 allocated - same
                  =~:        168 allocated - Infx more
```
Copy link
Copy Markdown
Owner

@grosser grosser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the java makes sense, but we don't need to optimize test runtime :)

@tagliala
Copy link
Copy Markdown
Contributor Author

tagliala commented Apr 22, 2026

Hi,

that is not for optimization

It's mentioned in the commit message

Also prefer match over =~ for more readable specs

- should ~=
+ should match

No problem in reverting the change btw

@grosser
Copy link
Copy Markdown
Owner

grosser commented Apr 22, 2026

hmmm true ... I like =~ more, but agree that match is easier to read and it's a good pattern to follow anyways

@grosser grosser merged commit fa1cc25 into grosser:master Apr 22, 2026
4 checks passed
@tagliala
Copy link
Copy Markdown
Contributor Author

Thanks

I noticed that there is production code related to JRuby, but JRuby itself is not currently tested in the CI pipeline.

I attempted to run the test suite locally using JRuby, but it appears not to be fully compatible at the moment, primarily due to the lack of support for fork, which JRuby does not implement.

I also experimented with a potential workaround using Copilot, but the result wasn’t entirely satisfactory and would likely need a more thoughtful approach.

Would you be open to considering adding JRuby to the CI matrix as a starting point? The suite is not fully passing yet, having visibility into failures could help guide incremental improvements and better understand the current gaps in compatibility.

If this sounds reasonable, I’d be happy to explore potential changes or contribute toward making the test suite more JRuby-friendly.

Thanks for your thoughts!

@grosser
Copy link
Copy Markdown
Owner

grosser commented Apr 23, 2026

yeah fine to add jruby and just make it skip anything with fork for example

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.

2 participants