3
\$\begingroup\$

I receive a string with a lot of characters that I don't need and I am trying to remove them and replace them with characters that I am able to work with. My current structure has me redefining the var multiple times which I feel is not efficient and can probably be done better. Please let me know of a more effective way I can do this.

I define the date then remove the unwanted characters then append the date to the "clean" string with and underscore:

var d = Date.now()
  var article = a.replace(/ |\./g, "_")
  article = article.replace(/\r?\n|\r/g,"")
  article = article.replace(/\$|\#|\[|\]/g, "")
  article = d + "_" + article

This does work but I am curious if there is a better way.

\$\endgroup\$
1
\$\begingroup\$

First, since you already did so inside of your two last expressions, with the same replacement:

article = article.replace(/\r?\n|\r/g,"")
article = article.replace(/\$|\#|\[|\]/g, "")

I'm puzzled why you didn't simply put both in a unique regexp:

article = article.replace(/\r?\n|\r|\$|\#|\[|\]/g, "")

Then to integrate with the 1st one, you might choose to:

  • join the two distincts replacements in a single line:

    var article = a.replace(/ |\./g, "_").replace(/\r?\n|\r|\$|\#|\[|\]/g, "")
    
  • or use a map approach, either suche the one pointed by @greybeard's link, or like this way (even if it might look a bit too sohpisticated for only two cases):

    var replacements = new Map([
        [/ |\./g, '_'],
        [/\r?\n|\r|\$|\#|\[|\]/g, '']
        ]),
        article = a;
    replacements.forEach(function(value, key){
          article = article.replace(key, value);
        });
    

The most interesting aspect in this latter solution is that it may be easily expanded if more replacements are needed.


EDIT following a good suggestion from @Niet the Dark Absol.

As soon as there are several unique characters to look for, with the same replacement, this kind of regexp /(a|b|c)/ can be replaced by /[abc]/, which is both simpler and more efficient!

Any of the above proposed solutions can be improved this way, so the latter one becomes:

    var replacements = new Map([
        [/[ .]/g, '_'],
        [/[\r\n$#[\]]/g, '']
        ]),
        article = a;
    replacements.forEach(function(value, key){
          article = article.replace(key, value);
        });
\$\endgroup\$
  • 1
    \$\begingroup\$ I would suggest improving that regex further. /[\r\n\[\]$#]/g is much more concise and works in the same way, and can have other options added to it easily enough. \$\endgroup\$ – Niet the Dark Absol Jan 23 '16 at 15:08
  • \$\begingroup\$ @NiettheDarkAbsol Indeed you're right, and I should have noticed that! I'll edit my answer about that. Thanks! \$\endgroup\$ – cFreed Jan 23 '16 at 16:13
1
\$\begingroup\$

You could define your pattern and replacement in an array. Then you can use reduce to carry the string through the array while replacing them.

let formatters= [
  {pattern: / |\./g, replacement: '_'},
  {pattern: /\r?\n|\r/g, replacement: ''},
  {pattern: /\$|\#|\[|\]/g, replacement: ''},
];

let article = formatters.reduce((a, f) => a.replace(f.pattern, f.replacement), a);
\$\endgroup\$
  • \$\begingroup\$ While this cleanly separates pairing patterns and replacements from using them in replace, it almost hides the time complexity of (avoidably) applying replace more than once. \$\endgroup\$ – greybeard Jan 23 '16 at 16:27

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.