3
\$\begingroup\$

I have a regex sequence that will take the end of a file path and select it for me:

resources/services/dealerInfoRequest/V5-0/dealerInfoRequestManager.cfc

In this, the regex would select:

/V5-0/dealerInfoRequestManager.cfc`

In some cases there is no V5-0 and it's just the file name and extension. My regex is working as shown but I was wanting to see if there was any optimization I could do, if it could be cleaned up, or if I should just leave it in the order it is now.

Regex is as shown:

((\/)(\w\d-?\d)(\/)\w.+)|(\/)(\w+\.\w+)

It's used in JavaScript and in Sublime Text 3.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ Try this: /([/][^/]+)?([/].+)$/ \$\endgroup\$
    – hjpotter92
    Commented Nov 14, 2014 at 4:36
  • \$\begingroup\$ @hjpotter92 Your regex is cool, but it matches too much. Check here: regex101.com/r/qP5jL4/1 \$\endgroup\$ Commented Nov 14, 2014 at 18:10
  • 1
    \$\begingroup\$ @IsmaelMiguel Sorry, replace .+ with [^/]+ \$\endgroup\$
    – hjpotter92
    Commented Nov 14, 2014 at 18:25
  • \$\begingroup\$ @hjpotter92 It works fine now, but it matches resources/services/dealerInfoRequest/f/dealerInfoRequestManager.cfc (replace /f/ by anything between / and it will match). The OP didn't specified if it is a requirement to keep the same format in the string. But I really like your solution. \$\endgroup\$ Commented Nov 14, 2014 at 19:20

2 Answers 2

3
\$\begingroup\$

Your regex has too many capturing groups.
Using (?:) makes it into a simple matching group, speeding up the regex since it doesn't have to store anything.

Since you are trying to get the part from the end of the string, anchor it there using $.

At the end, you have this: (\/)(\w+\.\w+).

This is really bad in 2 ways:

  1. You are using 2 capturing groups.
  2. You regex only allows files with letters. Spaces, underscores and others should de allowed too.

For version control, you have this: (\/)(\w\d-?\d).

This allows me to have a folder called t0-0 which makes no sense.
Be more strict about your match.

This is the regex I came up with:

\/(?:[vV][1-9]\d?(?:-\d)?\/)?[^\/]+\.[^\.]+$

It addresses all your issues.

Explaining each part:

  1. (?:\/[vV][1-9]\d?(?:-\d)?\/)? matches the version number, if there is any.
    1.1. No need for capturing groups.

    1.2. It enforces that the folder starts with v or V with.

    1.3. The version only can starts with 1-9 and can have any number or digits.

    1.4. The version folder is still optional

  2. [^\/]+\.[^\.]+$ matches the file name.
    2.1. Again: No need for capturing groups.

    2.2. The file can have ANY name.

    2.3. The file can have ANY extension, but there must be one.

  3. The whole string is matched to the end, which makes it easier for us to write it.
\$\endgroup\$
4
  • \$\begingroup\$ Wow thanks a bunch, I'm still pretty new to Regex, I was using a site called RegExr to help build what I got. I appreciate the clean up and the explanations. \$\endgroup\$
    – dhershman
    Commented Nov 11, 2014 at 16:40
  • \$\begingroup\$ @TimeDead You are welcome. A good website to test the regex is regex101.com which has a full explanation of every single part of your regex. I know mine isn't particularly simple to read but you should try it there. \$\endgroup\$ Commented Nov 11, 2014 at 16:47
  • \$\begingroup\$ oddly enough it doesn't pick up on either site: regex101.com/r/vF6aP5/1 However it works just fine via sublime etc. \$\endgroup\$
    – dhershman
    Commented Nov 11, 2014 at 16:53
  • \$\begingroup\$ @TimeDead I wasn't expecting it to be used for "batch-matching". On most text editors, each line is "seen" as a different string. On Javascript, there is no such a thing. However, splitting the string into an array using String.split('\r\n') and then matching each individually will solve the issue. But remember to use var regex=Regex('\/(?:[vV][1-9]\d?(?:-\d)?\/)?[^\/]+\.[^\.]+$'); instead of the literal regex. And the reason why it doesn't show anything in the matches is because there are no capturing groups. Try it here: gethifi.com/tools/regex (with only 1 filename) \$\endgroup\$ Commented Nov 11, 2014 at 17:02
2
\$\begingroup\$

I know, you thought, I will use regexes. Now you have a code review that bashes regexes.

All kidding aside, it would seem that this should suffice:

function filePathTrail( path ){
  var parts = path.split('/').slice(-2);
  return ( parts.length == 2 ? '/' : '' ) + parts.join('/');
}

///V5-0/dealerInfoRequestManager.cfc
filePathTrail('resources/services/dealerInfoRequest/V5-0/dealerInfoRequestManager.cfc');
//dealerInfoRequestManager.cfc
filePathTrail('dealerInfoRequestManager.cfc');
\$\endgroup\$
2
  • \$\begingroup\$ This is a nice solution for Javascript but, from what I understood, the OP also uses this for Sublime Text 3 (text-editor) which can't run Javascript (as far as I know). You are still right about this. I totally agree with you! But this only addresses half the problem. \$\endgroup\$ Commented Nov 14, 2014 at 18:18
  • \$\begingroup\$ Agreed, it is acceptable on CodeReview to only review part of the code/question. I claim no knowledge about regexes. Fortunately you have that covered. \$\endgroup\$
    – konijn
    Commented Nov 14, 2014 at 18:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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