RegEx version control

Question

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!

Welcome to Code Review! Please see What to do when someone answers. I have rolled back Rev 6 → 3 — Sᴀᴍ Onᴇᴌᴀ
– Sᴀᴍ Onᴇᴌᴀ ♦, Commented Jun 13, 2019 at 13:14

Sᴀᴍ Onᴇᴌᴀ · Accepted Answer · 2019-06-13 15:32:27Z

3

Likely logic flaw

In the first conditional:

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

Array.filter() "creates a new array"¹ 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.

¹_{https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/filter}

edited Jun 13, 2019 at 15:32

answered Jun 12, 2019 at 23:40

Sᴀᴍ Onᴇᴌᴀ♦

29.6k16 gold badges46 silver badges203 bronze badges

\$\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
– Hugo Cox

2019-06-13 11:19:51 +00:00
Commented Jun 13, 2019 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
– Hugo Cox

2019-06-13 11:38:50 +00:00
Commented Jun 13, 2019 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ᴇᴌᴀ
– Sᴀᴍ Onᴇᴌᴀ ♦

2019-06-13 15:35:58 +00:00
Commented Jun 13, 2019 at 15:35

Add a comment |

Hugo Cox · Accepted Answer · 2019-06-13 13:28:10Z

// 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:

let patternRegexMatches = pattern.match(regex); Only match 1x and only if there is a pattern submitted
pattern = (function() ... return pattern
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.

Stack Exchange Network

RegEx version control

2 Answers 2

Likely logic flaw

Improving efficiency

You must log in to answer this question.

Hot Network Questions

RegEx version control

2 Answers 2

Likely logic flaw

Improving efficiency

You must log in to answer this question.

Related

Hot Network Questions