Sorting a JavaScript array with a Regex

Question

I'm using SharePoint 2013 JSOM. My variable this.templates contains a collection of templates with many different names. The only way to iterate through this collection is by using an Enumerator. I can only get the name of each template by using get_title().

I want to get the templates whose names respect the following rules:

• It contains "espace_projet"
• It contains the letter "v" followed by any number, once

Then, out of the templates i got, I want to get the one with the highest value that was after the "v".

I'm not used to JavaScript, this code works fine, but feels like overkill. How could I make it shorter/more readable?

ApplyTemplate()
{
    var highestVersion = {number: -1, template: null};
    var templateEnum = this.templates.getEnumerator();

    while(templateEnum.moveNext())
    {
        //Gets the element name in my collection
        var templateName = templateEnum.get_current().get_title();
        if(templateName.includes('espace_projet'))
        {
            var version = templateName.match('v[0-9]+|v_[0-9]+');
            if(version){
                var versionNumber = version[0].match('[0-9]+');
                if(versionNumber[0] > highestVersion.number)
                    highestVersion = {number: versionNumber[0], template: templateEnum.get_current()};
            }
        }
    }

    return highestVersion;
}

Answer 1

You have it almost perfect in your question. With a little improvement you can reduce memory and CPU load.

1. Don't create a new object every time you find the next highest version. Set the properties of the existing object to the new highest.
2. Only call templateEnum.get_current() once, you should not call it again when you find the highest version
3. Don't pass strings to String.match it requires additional overhead to convert the string to a RegExp
4. Don't test for the same thing twice. .match('v[0-9]+|v_[0-9]+') and .match('[0-9]+') can be done in one step.

And some style points

1. The function is called ApplyTemplate (should not be capitalized) though to me it seams to find highest template. Maybe a better name?
2. The function is about templates, is there a need to add template to templateEnum, templateName Good naming uses the context to infer. Keep names short by not adding redundant and inferred details.

Thus your function becomes

highestVersionTemplate() {
    const highest = { number: -1 };
    const enumer = this.templates.getEnumerator();
    while (enumer.moveNext()) {
        const current = enumer.get_current();
        const title = current.get_title();
        if (title.includes("espace_projet")) {
            const ver = title.match(/v_?([0-9]+)/);
            if (ver && ver[1] > highest.number) {
                highest.number = Number(ver[1]);
                highest.template = current;
            }
        }
    }
    return highest;
},

Answer 2

Contentwise considerations

• Use const wherever possible, otherwise let. Avoid var.
• Match the version number only once (see code below) and employ the ? operator to save redundancy: v_?[0-9].
  (The latter might actually be disputable, I personally find both versions easy to read.)

Your code would be shorter and much easier to reason about if it was more functional. However, as JS is missing some functional concepts other languages take for granted (array comprehension, min/max in array by key function), this would require helper functions being a new, but onetime overhead. Are you using a third-party library like Lodash by accident? Then you already got many functional polyfills.

Which methods does the Enumerator you are using from SharePoint support apart from the iterative query interface with moveNext(), get_current()? If there is no built-in support, I'd suggest writing a generator function:

function *getTemplateIterator(templates) {
  const templateEnum = templates.getEnumerator();
  while (templateEnum.moveNext()) {
    yield templateEnum.get_current();
  }
}

function ApplyTemplate() {
  return [...getTemplateIterator(this.templates)]
    .filter(template => template.get_title().includes('espace_projet'))
    .map(template => {
      const version = /v_?([0-9]+)/.match(template.get_title());
      return {
        version: (version !== null) ? parseInt(version[1], 10) : -1
        template: template
      };
    })
    .reduce((highestVersionedTemplate, template) =>
      (template.version > highestVersionedTemplate) ? template : highestVersionedTemplate,
      { version: -1, template: null }
    );
    // In contrast with an array max/min function suporting a key extracting function
    // (e.g. like in Python: https://docs.python.org/3.8/library/functions.html#max)
    previousChainOfFunctions.maxOrDefault(template => template.version, { version: -1, template: null });
}

Stylistic considerations

• Your variable names are self-explanatory and consistently written, that's fine!

• Use consistent braces in blocks (function, if, many more), e.g. function { or function[newline]{, but do not mix both.

• Always use braces around if, else, for, while.