5
\$\begingroup\$

I have the following RegEx which matches any of a selection of inputs (see the list below).

While it works fine, I'm wondering if there is a better way of writing it, as it looks a little dirty. I've been using various RegEx builders but couldn't come up with a shorter version, so any tips would be appreciated.

-[0-9]+px$|^[0-9]+px$|^[0-9]+em$|^-[0-9]+em$|^[0-9]+\%$|^auto$|^0$

Here is the list of valid entries (where 10 can be any integer):

  • 10px
  • -10px
  • 10em
  • -10em
  • 10%
  • auto
  • 0
\$\endgroup\$

closed as unclear what you're asking by Jamal Aug 9 '15 at 20:28

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

14
\$\begingroup\$
  1. You can combine px and em into a single group

  2. You can pull in the negation sign with a ?

  3. [0-9] is equivalent (usually) to \d

  4. ^ and $ can be pulled out to the end (or use the api than only matches the entire string.

this results in:

^(-?\d+(px|em)|\d+%|auto|0)$

If you can have a negative % values then it's even simpler: ^(-?\d+(px|em|%)|auto|0)$

\$\endgroup\$
  • \$\begingroup\$ Looks like it's supposed to accept CSS values, so negative percentages should be okay. But at the same time it'd be missing pt as a unit. \$\endgroup\$ – Mario Apr 29 '15 at 14:12
  • \$\begingroup\$ @ratchetfreak - Thank you, that works perfectly. I was fairly confident that my work could be shortened, but I didn't think it could be this compressed. \$\endgroup\$ – David Gard Apr 29 '15 at 14:22
  • 1
    \$\begingroup\$ @Mario - It is indeed to check CSS values, so I have rolled % in with px and em, as well as adding pt as you suggest. I'll propably also change it to accept any integer value (\d) rather than just 0, then check using PHP is_numeric() and add px as a default. \$\endgroup\$ – David Gard Apr 29 '15 at 14:26
  • \$\begingroup\$ I'd suggest two additional changes to the 'final' version of the regex. First off I'd make the digit quantifier posessive to improve backtracking when confronted with failing cases, and secondly I'd change the unit capture group into a non-capturing group to clarify the intent of the regex. Including these changes one arrives at: ^(-?\d++(?:px|em|%)|auto|0)$ another optimization might be to make the unit optional to eliminate the special case for 0 \$\endgroup\$ – Vogel612 Apr 29 '15 at 23:08
  • \$\begingroup\$ @Vogel612 actually depending on how it gets used I'd explicitly make the unit and also the number capturing so it can be extracted and used after matching. \$\endgroup\$ – ratchet freak Apr 29 '15 at 23:12
2
\$\begingroup\$

I'd suggest that you simplify. Imagine you're a junior programmer and you are asked to edit this abomination (or the one in the accepted answer) in order to add support for inputs of the type 10cm and -10cm:

-[0-9]+px$|^[0-9]+px$|^[0-9]+em$|^-[0-9]+em$|^[0-9]+\%$|^auto$|^0$

Would you be able to do that without introducing false positives or false negatives? How many test cases would you need to pass to be sure? Why not try this approach:

public bool IsLength(string arg) {
    return IsLengthPixel(arg) || IsLengthEm(arg) || IsLengthPercent(arg) ...;
}

public bool IsLengthPixel(string arg) {
    return IsMatch(arg, "^-?[0-9]+px$");
}

In this way you can add IsLengthCm(string) without screwing up any of the existing length checks. It's much easier to read this code and, with good naming conventions, it documents itself.

\$\endgroup\$
  • \$\begingroup\$ In other words don't add the problem of regex to this one. Instead explicitly parse each option separately. \$\endgroup\$ – ratchet freak Apr 29 '15 at 19:31

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