1
\$\begingroup\$

I have this conditional statement with use of regex:

do_stuff if (@string.match(/<(?:ingr?)>/i) && 
            @string.match(/<(?:prod:alchemy?)>/i)) ||
            @string.match(/<(?:key?)>/i)

It's a 1-line conditional statement, but due to it's length, it taking 3 lines instead. I'm wondering if there's a way to trim this down, so that I can fit it into 1 line. Preferably to be something like

do_stuff if (regex here)
\$\endgroup\$
  • 1
    \$\begingroup\$ I am not clear why you need the non-capture groups and why they need to be non-greedy (the ? at the end of each). If, for example, you are just trying to see if the string '<ingr>' is present, you could use just @string.include?('<ingr>'). Could you please elaborate? It would be helpful if you could edit to include some example strings and expected results. \$\endgroup\$ – Cary Swoveland Mar 16 '14 at 5:57
  • 1
    \$\begingroup\$ The question marks here aren't lazy quantifiers. They simply make the preceding letter optional. (I'm not sure if that was that the author intended, but that's what it does.) \$\endgroup\$ – 200_success Mar 16 '14 at 8:19
  • \$\begingroup\$ Thanks, @200_success. I see a ? implies lazy (aka "non-greedy", "reluctant") only when if follows one of the qualifiers * , + , ? , or {}. My regex skills need work. \$\endgroup\$ – Cary Swoveland Mar 17 '14 at 0:55
3
\$\begingroup\$

You could combine all the tests into one regular expression, but I don't recommend it. The following is equivalent to what you wrote:

do_stuff if @string =~ /(?'i'<ingr?>).*(?'a'<prod:alchemy?>)|\g<a>.*\g<i>|<key?>/i

Your regular expressions are a bit odd, so I suspect that what you wrote is not what you meant to write.

The non-capturing grouping parentheses (?:…) don't seem to serve any purpose, so you can get rid of them.

The other question marks make the preceding letter optional. For example, /<key?>/i matches <key> or <Key> or <ke> but not <ken>.

The tricky obstacle to merging the regular expressions is that the <ingr?> and <prod:alchemy?> tests both have to pass, and they can occur in either order. Therefore, you have to allow <ingr?>.*<prod:alchemy?> as well as <prod:alchemy?>.*<ingr?> That's possible with

<ingr?>.*<prod:alchemy?>|<prod:alchemy?>.*<ingr?>

Of course, repeating yourself is bad for verbosity and maintainability. You can lessen the pain slightly by using named groups and subexpression calls.

After having done all that, though, the one long regex is more difficult to understand than your original code. Therefore, I recommend leaving it alone.

If you're dissatisfied with the original code, though, there may be other improvements that you could make, but you haven't provided any context for your question so I can't make alternative suggestions.

\$\endgroup\$
  • \$\begingroup\$ Ah, thank you. Your feedback is more than helpful. Especially that sub-expression call. I saw what you did there with \g<a>.*\g<i>. Thanks! \$\endgroup\$ – user3163916 Mar 16 '14 at 19:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.