2
\$\begingroup\$

Basically I have an url which looks like this structurally speaking:

http://www.my-site.com/topic1/topic2/topic3/topic4/topic5

and I want to do 2 things to it:

1. check the structure and validate it
2. replace both topic3 and topic4 with a new parameters called topic6

This is my solution for now but I am interested to build something more optimize.

if ((url.match(/\//g) || []).length === 7) {
    const arr = url.split('/');
    const topic5 = arr.pop();
    arr.pop();
    arr.pop();
    arr.push('topic6', topic5);
    return arr.join('/');
}

So as you can see, for the fact that I am not very good at regular expressions I have used another approach which doesn't look that good. I basically check for the number of / in the string, if tey are 7 it means that the structure of the url is good and that on this type of url I should apply the next steps. After that I grab the last param of the url and also remove it and next after that I remove the last 2 parameters and add the new one instead along with the topic5.

In case you you have a better approach, please let me know. I think that it can be done by writing less number of lines of code but as I said, I am not very familiar with regular expressions.

\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to CR! The requirements for the function are unclear to me. What sort of characters constitute the placeholders for topic1, topic2 etc? Are they all alphanumeric? Do we always have 5? Counting 7 slashes seems brittle--what if https:// is omitted? Please provide a variety of valid and invalid examples with explanation for each one as to why it's valid/invalid. Additionally, why do you need to optimize this? I'm surprised if you're running into performance issues on strings that are just a few characters long. Please elaborate. What is the "slip approach"? Thanks! \$\endgroup\$ – ggorlen Sep 21 at 16:20
  • \$\begingroup\$ topic1, topic2, etc are slugs, can contain also numbers, - sign but also diacritics from different languages or cyrilic chars. We don't have always 5 because the website have different type of urls but I must detect only the ones which have 5 and do these changes on them. I want to optimize not necessary for performance but for write better code which has more quality. \$\endgroup\$ – paulalexandru Sep 21 at 17:56
  • \$\begingroup\$ Thanks for the clarifications, that helps! Can you provide some sample inputs/outputs? \$\endgroup\$ – ggorlen Sep 21 at 18:55
  • \$\begingroup\$ Input: my-site.com/topic1/topic2/topic3/topic4/topic5 ; Output: my-site.com/topic1/topic2/topic6/topic5 \$\endgroup\$ – paulalexandru Sep 24 at 5:43
  • 1
    \$\begingroup\$ Please add relevant details to the question itself as an Edit. This puts the full story all in one place. Volunteers don't want to have to read a running dialogue in comments to fully understand the scope of the code to be reviewed. \$\endgroup\$ – mickmackusa Sep 24 at 21:50
1
\$\begingroup\$

You seek to:

  1. validate the url as coming from your site domain, then
  2. replace the 3rd and 4th directories/topics with a single/new directory/topic.

This is a simple matter of preserving the substrings that you want to keep as "capture groups" and writing the new replace substring between the two capture groups.

In my demo, the new topic will be FOO. I'm using negated character classes to match the directory names -- this is relatively "loose" validation. If you require a tighter validation rule, see the 2nd snippet. Click on Run code snippet below to see the resultant output.

const regex = /^((?:https?:\/\/)(?:w{3}\.)?my-site\.com\/(?:[^/]+\/){2})[^/]+\/[^/]+(\/[^/]+)$/;
      url = 'http://www.my-site.com/topic1/Topic2/topic3/topic4/слизень',
      replacement = '$1FOO$2';

document.write(url.replace(regex, replacement));

To tighten the validation beyond non-slashes as directory substrings AND extend the characters classes to include multibyte letters, you will need to manually expand the character classes to include the letters that you expect to qualify. Please have a read of Regular expression with the cyrillic alphabet.

const regex = /^((?:https?:\/\/)(?:w{3}\.)?my-site\.com\/(?:[À-žа-яa-z\d-]+\/){2})[À-žа-яa-z\d-]+\/[À-žа-яa-z\d-]+(\/[À-žа-яa-z\d-]+)$/i;
      url = 'http://www.my-site.com/Schnecke/to-pic2/topic3/naaktslak/слизень',
      replacement = '$1FOO$2';

document.write(url.replace(regex, replacement));

\$\endgroup\$
0
\$\begingroup\$

Your codes looks pretty good, even though (?:I'm not really a code reviewer])*.

If you wish to use regular expressions though for validations, there are several strategies that you can follow up, given that validations with regular expressions are a bit difficult to do and it'd be good to know some details of the boundaries and limitations.

My guesswork is that maybe here we would start with some expression similar to:

^((?:https?:\/\/)(?:w{3}\.)?my-site\.com(?:\/[a-z0-9]{1,64}){2}\/)[a-z0-9]{1,64}(\/[a-z0-9]{1,64}\/)[a-z0-9]{1,64}(\/?)$

Demo

which has some quantifiers to just limit the sizes, if that'd be desired, and since we'd be replacing the third and fifth topics, then we'd define our replacement similar to,

$1some_other_topic_3$2some_other_topic_5$3

for instance.


There would be many other options that you could modify your expression with, such as,

^((?:https?:\/\/)(?:w{3}\.)?my-site\.com(?:\/[^\/\r\n]+){2}\/)[^\/\r\n]+(\/[^\/\r\n]+\/)[^\/\r\n]+(\/?)$

which would depend on how you'd like to validate the URLs.

Demo 2

const regex = /^((?:https?:\/\/)(?:w{3}\.)?my-site\.com(?:\/[^\/\r\n]+){2}\/)[^\/\r\n]+(\/[^\/\r\n]+\/)[^\/\r\n]+(\/?)$/gmi;
const str = `http://www.my-site.com/topic1/topic2/topic3/topic4/topic5
https://www.my-site.com/topic1/topic2/topic3/topic4/topic5
http://my-site.com/topic1/topic2/topic3/topic4/topic5
https://my-site.com/topic1/topic2/topic3/topic4/topic5
http://www.my-site.com/topic1/topic2/topic3/topic4/topic5/
https://www.my-site.com/topic1/topic2/topic3/topic4/topic5/
http://my-site.com/topic1/topic2/topic3/topic4/topic5/
https://my-site.com/topic1/topic2/topic3/topic4/topic5/
https://notmy-site.com/topic1/topic2/topic3/topic4/topic5/`;
const subst = `$1some_other_topic_3$2some_other_topic_5$3`;


const result = str.replace(regex, subst);

console.log(result);


If you wish to simplify/modify/explore the expression, it's been explained on the top right panel of regex101.com. If you'd like, you can also watch in this link, how it would match against some sample inputs.


RegEx Circuit

jex.im visualizes regular expressions:

enter image description here

\$\endgroup\$

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.