4
\$\begingroup\$

this is my first time on this stack exchange :)

I created a little script to check for the latest version of specified API version available, but I am wondering if I am using the best way possible for checking regex and includes?

The var arr will be filled dynamically with version folders using fs.readdir, but for simplicity I wrote them in the array manually and the var pattern will come from req.params.v using nodejs express router middleware.

Just looking for an approval or if there is a better way, thank you!

var arr = ['v1.0.0', 'v5.2.4', 'v5.2.9', 'v5.20.4', 'v6.4.0'];
var pattern = 'v5.2';

const regex = /^v[0-9](\.[0-9]{1,2})?(\.[0-9])?$/;

if (regex.test(pattern) && (arr.filter(s => s.includes(pattern)) != 0)) {

    if (pattern.match(/\./g) == null) {
        console.log('major version');
        console.log(pattern);
    } else if (pattern.match(/\./g).length == 1) {
        console.log('minor version');
        console.log(pattern);
        pattern = pattern + '.';
    } else {
        console.log('patch version');
        console.log(pattern);
    }
    const matches = arr.filter(s => s.includes(pattern));
    console.log(matches[matches.length - 1]);

} else {
    console.log('Specify correct API version!');
}

Thank you in advance!

\$\endgroup\$
3
\$\begingroup\$

Likely logic flaw

In the first conditional:

if (regex.test(pattern) && (arr.filter(s => s.includes(pattern)) != 0)) {

Array.filter() "creates a new array"1 so the second conditional expression compares an array with zero. Those two should never be loosely equal so that conditional always evaluates to true. Perhaps you intended to check the length property of the array.

Improving efficiency

The code has repeated operations - e.g. arr.filter(s => s.includes(pattern)) appears twice, and then there are multiple calls to pattern.match(). The call to arr.filter() could be stored in a variable and then used whenever needed instead of re-filtering.

Also, the regular expression stored in regex could be used with String.match() to get an array of matches. Because the regular expression has capturing groups, those capturing groups can be used instead of calling pattern.match(/\./g) multiple times to check for the number of dots. Instead, check if the array returned by String.match() isn't null and then if the 2nd and 3rd elements are undefined.

See a comparison in this jsPerf.

1https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/filter

\$\endgroup\$
  • \$\begingroup\$ There is a flaw in your code, namely specifying v5.2 should return the latest of minor version 2, namely v5.2.9 and not v5.20.4 (minor version 20) \$\endgroup\$ – Hugo Cox Jun 13 at 11:19
  • \$\begingroup\$ I have updated my code, see edit. According to jsperf (thank for you that website!) it is the fastest way. \$\endgroup\$ – Hugo Cox Jun 13 at 11:38
  • \$\begingroup\$ Darn I missed that part about updating the pattern when there is a minor version detected. I have removed that snippet since it isn't the same. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Jun 13 at 15:35
0
\$\begingroup\$
// const fs = require('fs');
// const folder = "../api/";
const regex = /^v[0-9](\.[0-9]{1,2})?(\.[0-9])?$/;

// let files = fs.readdirSync(folder);
let pattern = "v5.2"; // let pattern = req.params.v;

var arr = ['v1.0.0', 'v5.2.4', 'v5.2.9', 'v5.20.4', 'v6.4.0']; // var arr = [];

/** Function **/
/*
function naturalSort(a, b) {
    var ax = [],
        bx = [];

    a.replace(/(\d+)|(\D+)/g, function (_, $1, $2) {
        ax.push([$1 || Infinity, $2 || ""])
    });
    b.replace(/(\d+)|(\D+)/g, function (_, $1, $2) {
        bx.push([$1 || Infinity, $2 || ""])
    });

    while (ax.length && bx.length) {
        var an = ax.shift();
        var bn = bx.shift();
        var nn = (an[0] - bn[0]) || an[1].localeCompare(bn[1]);
        if (nn) return nn;
    }

    return ax.length - bx.length;
}

files.forEach(file => {
    let dirStat = fs.statSync(folder + '/' + file).isDirectory();
    if (dirStat) {
        arr.push(file);
    }
});

arr.sort(naturalSort);
*/
/**/

if (regex.test(pattern)) {

    pattern = (function () {
        let patternRegexMatches = pattern.match(regex);

        if (patternRegexMatches[1] === undefined) {
            console.log('major version');
            console.log(pattern);
        } else if (patternRegexMatches[2] === undefined) {
            console.log('minor version');
            console.log(pattern);
            pattern = pattern + '.';
        } else {
            console.log('patch version');
            console.log(pattern);
        }

        return pattern;
    })();

    let matches = arr.filter(s => s.includes(pattern));

    if (matches.length != 0) {
        console.log(matches[matches.length - 1]);
    } else {
        console.log('Specify correct API version!');
    }

} else {
    console.log('Specify correct API version!');
}

Thanks to the review, I updated my code with the following improvements:

  1. let patternRegexMatches = pattern.match(regex); Only match 1x and only if there is a pattern submitted
  2. pattern = (function() ... return pattern
  3. Only 1x arr.filter(s => s.includes(pattern));

Additionally, in javascript the folder structure is not "naturally sorted" as with ls -v on linux, so I added an additional function to make sure the version folders are sorted correctly.

\$\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.